[Edbrowse-dev] Error Legs

[Edbrowse-dev] Error Legs

[Karl Dahlke]

Ok, I follow what you're saying.

The expedient approach is to wrapper some of the js calls,
the way I wrapper malloc, to simply exit upon null,
and I think that would be fine for now, provided the runtime pool
is pushed up to 64 meg.
It just wouldn't come up very often.
People download huge files, but that doesn't consume huge javascript space.
Web pages come in a certain size, and usually don't include memory intensive js.

Later versions can, perhaps, be more graceful about these allocation errors,
but I think size_t issues, better dom support,
ajax, plugins (particularly flash), and imap, are all higher priority.
For now, a version that never ever segfaults would be lovely.

Karl Dahlke

Re: [Edbrowse-dev] Error Legs

[Chris Brannon]

Karl Dahlke <eklhad@comcast.net> writes:

> The expedient approach is to wrapper some of the js calls,

It's even easier than that.  my_ErrorReporter gets a pointer to
JSErrorReport, which will tell us exactly what went wrong.  So we can
exit on out-of-memory inside my_ErrorReporter.  No wrappers, no other
error legs needed, I think.
Do a pull, and you'll see what I mean.

Now here are the open questions.  Do we need to be exiting on conditions
other than out-of-memory?  Maybe we do, and we don't know what they are
yet...

Also, this strategy is fine when a native C function, such as domLink,
fails to allocate.  However, if the out-of-memory condition happens in a
script, passed to JS_EvaluateScript, it is converted to a JavaScript
exception.  my_ErrorReporter will get called to indicate the failure,
but the failure will be converted to "uncaught exception: out of
memory", indicating that the script failed to catch the exception.
my_ErrorReporter will see an out-of-memory condition the next time a C
function fails to allocate.  Is this good enough?
Ideally, I'd like to know about out-of-memory during script evaluation,
rather than having it silently converted to a JS exception, but I don't
think we can do that.

Look  at my http://the-brannons.com/array.html example if the previous
paragraph is unclear.

-- Chris

[Edbrowse-dev] Error Legs

[Karl Dahlke]

> exit on out-of-memory inside my_ErrorReporter.

Very good.
I pushed a small change to use the symbol and message we already had for this
condition, rather than making a new one.
I try to avoid extra messages, as they have to be translated.
When you do add a message though, append zeros for the other languages,
just so I can keep track of outstanding messages that haven't yet been
translated into this or that.

Yes, as you point out we could exit on

if(stringEqual(message, "out of memory")

I'm thinking maybe not though.
Marching on won't lead to a segfault,
and as Adam notes, another buffer may have a lot of work in it,
so why exit.
It's not worse or better than the other js errors,
like undefined variable.
Of course, if truly out of memory,
the next js get object call will probably fail,
and that one will probably exit.
So maybe

if(stringEqual(message, "out of memory")) {
print some helpful warning about memory and risk of exit
and saving your buffers.
}

Your demo program is simple,
but I would point out that it doesn't fail if you replace {} with 37.
An array with a million elements still fits,
but an array of a million arrays, with all the associated overhead
of all those objects, that doesn't fit.
Hard to imagine a web page making a million arrays.
So it makes me wonder if we ever bump up against this limit
on a real website.
Are we jumping at shadows, or is this really an issue?
If 4 meg isn't enough, 16y probably is.
My gut tells me the segfaults might come from somewhere else.
Then again, you could have many web pages, with many
contexts and pages, in one edbrowse session.
IDK


Karl Dahlke

Re: [Edbrowse-dev] Error Legs

[Chris Brannon]

Karl Dahlke <eklhad@comcast.net> writes:

> I pushed a small change to use the symbol and message we already had for this

Oh, right.  You forgot to modify jsdom.cpp though.
Fixed and pushed.

> My gut tells me the segfaults might come from somewhere else.

Here's a transcript.
This is with the 4 meg size for JS_NewRuntime and the new change to my_ErrorReporter.

Script started on Fri 07 Feb 2014 10:15:32 AM PST
./edbrowse http://www.youtube.com
..
230809
13578
/Popu
* {Thumbnail Popular on YouTube} 
g
.....
508101
JavaScript failure
chris@mushroom: /files/cmb-files/repos/edbrowse/src $ exit
exit

Script done on Fri 07 Feb 2014 10:15:55 AM PST

So it is failing with out-of-memory in this case.  I'm sure youtube is a
very JS-intensive site, but could there be another reason for the memory
exaustion?  I don't know.

-- Chris

Re: [Edbrowse-dev] Error Legs

[Adam Thompson]

On Fri, Feb 07, 2014 at 10:23:05AM -0800, Chris Brannon wrote:
> Karl Dahlke <eklhad@comcast.net> writes:
> > My gut tells me the segfaults might come from somewhere else.
> So it is failing with out-of-memory in this case.  I'm sure youtube is a
> very JS-intensive site, but could there be another reason for the memory
> exaustion?  I don't know.

Not sure, we really need to go through everything and ensure that what needs to
be rooted is and what doesn't isn't.
Incidentally I bumped my jspool limit up to 64 meg and that example works now.
Really it'd be much nicer if we could get js to dynamically grow to deal with
this (with an *optional* limit).

I'd still prefer it if it just killed the offending context as well
(js_DestroyContext and js_GC?). This should mean that everything else can
continue and we reclaim the lost memory,
or in the worst case just the js side is broken.
Either way, it allows the user to save critical work, bookmark pages etc.
At the moment, although I agree that not segfaulting is much nicer,
the overall outcome is the same; edbrowse goes away taking your unsaved work
and other browsing sessions with it.
I can try and implement this if people agree.

Cheers,
Adam.

Re: [Edbrowse-dev] Error Legs

[Adam Thompson]

On Sun, Feb 09, 2014 at 10:45:25AM +0000, Adam Thompson wrote:
> On Fri, Feb 07, 2014 at 10:23:05AM -0800, Chris Brannon wrote:
> > Karl Dahlke <eklhad@comcast.net> writes:
> > > My gut tells me the segfaults might come from somewhere else.
> > So it is failing with out-of-memory in this case.  I'm sure youtube is a
> > very JS-intensive site, but could there be another reason for the memory
> > exaustion?  I don't know.
> 
> Not sure, we really need to go through everything and ensure that what needs to
> be rooted is and what doesn't isn't.
> Incidentally I bumped my jspool limit up to 64 meg and that example works now.
> Really it'd be much nicer if we could get js to dynamically grow to deal with
> this (with an *optional* limit).
> 
> I'd still prefer it if it just killed the offending context as well
> (js_DestroyContext and js_GC?). This should mean that everything else can
> continue and we reclaim the lost memory,
> or in the worst case just the js side is broken.
> Either way, it allows the user to save critical work, bookmark pages etc.
> At the moment, although I agree that not segfaulting is much nicer,
> the overall outcome is the same; edbrowse goes away taking your unsaved work
> and other browsing sessions with it.
> I can try and implement this if people agree.

Looking at the docs it looks like this can't be done inside the error reporter,
and at any rate, I've still seen (whilst trying to browse the docs)
more js-related segfaults (with no helpful errors though).

All this is pushing me back to doing the error leg stuff properly rather than in the error reporter.
That, and the fact that according to the docs,
any function which returns a value and takes a js context pointer can
(depending on return) return either false or NULL to indicate failure.
I assume by this they mean they're not going to make explicit the failure
conditions for these functions, just that failure is always an option in these
cases.
They do also say that such failures aren't always due to out of memory
conditions, and may be due to such things as syntax errors etc.

Cheers,
Adam.

[Edbrowse-dev] Error Legs

[Karl Dahlke]

youtube example...
It runs fine for me, no memory error.
IDK

Karl Dahlke

Re: [Edbrowse-dev] Error Legs

[Chris Brannon]

Karl Dahlke <eklhad@comcast.net> writes:

> youtube example...
> It runs fine for me, no memory error.

You're still running a 32-bit machine / OS, right?  I run 64-bit, so
maybe the difference is pointer size.

-- Chris

[Edbrowse-dev] Error Legs

[Karl Dahlke]

> You're still running a 32-bit machine / OS, right? I run 64-bit, so
> maybe the difference is pointer size.

Well if that relatively small difference could push you over the edge,
then likely I would get the memory error if I change my runtime pool
from 4 meg to 2 meg; and I do.
Therefore, real world websites can consume 4 meg js;
and I do remember youtube use to cause a lot of the segfaults.
I suggest we push the size up to 16 meg, or perhaps 32.

Karl Dahlke

[Edbrowse-dev] Error Legs

[Karl Dahlke]

> I'd still prefer it if it just killed the offending context as well

I agree, and I might be the one to work on this -
but that would be one of those major changes that I think we said we would
wait on until our work was stable.
I keep coming back round to the fact that if we can't get this to work
with moz js 24 libraries as distributed, then edbrowse is dead anyways,
except perhaps on our three machines.
If it can't be distributed then I don't know how much time I should spend on it.
I'm not trying to be Debbie Downer, just practical.
We should be doing what we can to get it to function in the debian world.
If that is both in process and/or on hold,
or if one of you is taking that on, then I could work on error legs in parallel,
and would be willing to do that, as it is a systemic change throughout.
I just keep looking up and seeing the sword of damocles overhead.

Karl Dahlke

Re: [Edbrowse-dev] Error Legs

[Adam Thompson]

On Sun, Feb 09, 2014 at 09:18:46AM -0500, Karl Dahlke wrote:
> > I'd still prefer it if it just killed the offending context as well
> 
> I agree, and I might be the one to work on this -
> but that would be one of those major changes that I think we said we would
> wait on until our work was stable.
> I keep coming back round to the fact that if we can't get this to work
> with moz js 24 libraries as distributed, then edbrowse is dead anyways,
> except perhaps on our three machines.
> If it can't be distributed then I don't know how much time I should spend on it.
> I'm not trying to be Debbie Downer, just practical.

Hmmm, it breaks with *one* distributed package of mozjs 24,
and not even a stable one. The fact is that it works with mozjs as distributed
by mozilla, which tells me that it works well enough for distribution.

> We should be doing what we can to get it to function in the debian world.
> If that is both in process and/or on hold,
> or if one of you is taking that on, then I could work on error legs in parallel,
> and would be willing to do that, as it is a systemic change throughout.

You're right that we *need* to work out how to get this to function with the
debian package, and I think both Chris and I are working on this.
I think it'd be helped greatly by improved error handling though,
as it *may* be that the bug with debian is a reflection of something wrong in
edbrowse's use of the lib which is showing up in other strange problems when
run against hand-compiled versions.
At the end of the day, if we know the browser is properly stable with one
version of the lib it'll make it easier to find where the instability is with
the debian package. At the moment, I 'm fairly sure that the bug is something
to do with not defining something or something missing in our initialisation or
the Debian package. However, when the thing segfaults for seemingly no reason
sometimes, there's always the suspician that something else is going on.

> I just keep looking up and seeing the sword of damocles overhead.

I think this is being a bit over-dramatic.
The cleanup for the new release and the stabilisation process is painful,
but certainly not fatal for the project.
I'm not surprised about this, there are things which need to be fixed and the
changes in the mozilla api are signifficant.
Debian are still packaging edbrowse so it's not gone from their repo and is
still in stable. As long as we can show them there's development this will
hopefully be ok. I think we need to make the release relatively soon,
but I think waiting for error handling,
whilst perhaps pushing the concept of code cleanup a bit, makes sense.
After all, what'll kill the project is a bug-filled release, not a delayed one.
If people view edbrowse as a segfault wating to happen then they'll stop using it.
If they have to wait a little longer for a major new release then they'll
probably be fine with that as long as they know why.

Cheers,
Adam.

[Edbrowse-dev] Error Legs

[Karl Dahlke]

It is important not to stop javascript, even the current context,
on a simple error like a syntax error or unreferenced variable.
This happens a lot; some kind of analytics script
that just tracks visitors to the site,
and those tend to be complicated javascript,
so uses a function that I don't have, and stops,
but then the next chunk of code is executed,
and that is the heart of the website, code that we really need to function.
So this is one of those things I should think about for a couple days,
before writing a line of code.

Karl Dahlke

Re: [Edbrowse-dev] Error Legs

[Adam Thompson]

On Sun, Feb 09, 2014 at 09:48:57AM -0500, Karl Dahlke wrote:
> It is important not to stop javascript, even the current context,
> on a simple error like a syntax error or unreferenced variable.
> This happens a lot; some kind of analytics script
> that just tracks visitors to the site,
> and those tend to be complicated javascript,
> so uses a function that I don't have, and stops,
> but then the next chunk of code is executed,
> and that is the heart of the website, code that we really need to function.
> So this is one of those things I should think about for a couple days,
> before writing a line of code.

Yeah, I agree. I guess my point is more that when we have an unrecoverable
error we need to stop running the current script and possibly clean up the 
context.
However I really think we shouldn't allow js to kill the entire browser,
particularly if we want to get more people to use it.

Is there a way of killing the current script and then moving on to the next one?
The docs said something about throwing an uncatchable error within a c function?

Cheers,
Adam.