[Edbrowse-dev] suggest increasing the size argument to JS_NewRuntime

[Edbrowse-dev] suggest increasing the size argument to JS_NewRuntime

[Karl Dahlke]

> From what I can tell, the remaining segfaults all seem to be caused by
> out-of-memory errors in JS.

Really?
JS is so bad that when it exhausts its memory pool it segfaults?
That doesn't give me a warm fuzzy feeling.

Or maybe js returns 0, like any malloc failure,
and I don't watch for that and then I segfault.
If that is the case then we need to write lots more error legs,
which is really tedious programming.

Sure, push the run time memory up to 32 or 64 meg. Why not?
Probably can't buy a computer these days with under 4 gig,
or even a smart phone.
But that doesn't explain why Chris simple program segfaults on debian js.

> As an aside, with debug level 2 or greater,
> edbrowse will print "out of memory" before it crashes.

Not sure what this means.
My wrappers around malloc do indeed print and exit upon malloc failure,
which probably saves me about 3,000 lines of error leg programming,
but you don't need debug levels, it just happens.
However ... on a 64 bit machine with lots of memory,
integers could overflow before we are out of ram.
Even though it's a signed int, malloc prototype is size_t, which is unsigned,
so we're really good all the way up to unsigned in, however,
beyond that we are allocating a really small number,
instead of 4 gig, and then we tromp over unallocated memory, and that's bad.
Remember that edbrowse was originally written when the best computer
on the market had 64 meg.
So I need to revamp all these strings and allocated arrays, and use size_t,
and then for the 32 bit machines, make sure there is no size_t overflow.
A file with half a billion lines, times sizeof(struct lineMap),
goes over the limit.
Plenty to think about here, but perhaps more important is
putting in more dom objects and functions to support more websites.
But we should probably remain focused on js 24 and the next release.

Thank you so much for all your help and hard work.

Karl Dahlke

Re: [Edbrowse-dev] suggest increasing the size argument to JS_NewRuntime

[Adam Thompson]

On Fri, Feb 07, 2014 at 01:30:51AM -0500, Karl Dahlke wrote:
> > From what I can tell, the remaining segfaults all seem to be caused by
> > out-of-memory errors in JS.
> 
> Really?
> JS is so bad that when it exhausts its memory pool it segfaults?
> That doesn't give me a warm fuzzy feeling.
> 
> Or maybe js returns 0, like any malloc failure,
> and I don't watch for that and then I segfault.
> If that is the case then we need to write lots more error legs,
> which is really tedious programming.

This is probably the case.

> Sure, push the run time memory up to 32 or 64 meg. Why not?
> Probably can't buy a computer these days with under 4 gig,
> or even a smart phone.
> But that doesn't explain why Chris simple program segfaults on debian js.

No it doesn't, I've got a feeling that's a broken debian package.
I'm going to look at this more over the weekend.

> > As an aside, with debug level 2 or greater,
> > edbrowse will print "out of memory" before it crashes.
> 
> Not sure what this means.
> My wrappers around malloc do indeed print and exit upon malloc failure,
> which probably saves me about 3,000 lines of error leg programming,
> but you don't need debug levels, it just happens.
> However ... on a 64 bit machine with lots of memory,
> integers could overflow before we are out of ram.
> Even though it's a signed int, malloc prototype is size_t, which is unsigned,
> so we're really good all the way up to unsigned in, however,
> beyond that we are allocating a really small number,
> instead of 4 gig, and then we tromp over unallocated memory, and that's bad.

Yeah, that's certainly unfortunate.

> Remember that edbrowse was originally written when the best computer
> on the market had 64 meg.
> So I need to revamp all these strings and allocated arrays, and use size_t,
> and then for the 32 bit machines, make sure there is no size_t overflow.

May as well do this for all machines, or just not bother.
On 32 bit machines, can you really get enough ram to exhaust 32 bit size_t?

> A file with half a billion lines, times sizeof(struct lineMap),
> goes over the limit.
> Plenty to think about here, but perhaps more important is
> putting in more dom objects and functions to support more websites.
> But we should probably remain focused on js 24 and the next release.

I think keep the focus on js 24 for the minute,
with a focus on the size_t change after the release.
At the end of the day if the back end is stable then adding dom objects etc is
much easier. Also, as long as the api remains somewhat the same (with int
switched for size_t) people can be working on both things at once.

> Thank you so much for all your help and hard work.

Thanks for providing such a useful program.

Cheers,
Adam.

Re: [Edbrowse-dev] suggest increasing the size argument to JS_NewRuntime

[Chris Brannon]

Karl Dahlke <eklhad@comcast.net> writes:

> Or maybe js returns 0, like any malloc failure,
> and I don't watch for that and then I segfault.

Yes, this is correct.  My wording was loose.  JS returns NULL somewhere
because of an out of memory condition, and the program eventually
segfaults because that NULL is passed along to a function that cannot
deal with it.
As you say, we need lots more error legs.  I think we've had that
discussion, and no one has written them yet.
So I'll get started on those today.

> But that doesn't explain why Chris simple program segfaults on debian js.

Yes, that's a different issue entirely. My program on Debian doesn't 
even make it past JS initialization.  It fails before it can even get a
JS context.

> My wrappers around malloc do indeed print and exit upon malloc failure,
> which probably saves me about 3,000 lines of error leg programming,

No, when Spidermonkey runs out of memory, the "out of memory" message is
printed via my_ErrorReporter() from jsdom.cpp.
E.G., try browsing http://the-brannons.com/array.html with db2.

We could write another set of wrappers for JS functions.  E.G.,
our_JS_NewObject, which causes an exit when JS_NewObject fails.

-- Chris

Re: [Edbrowse-dev] suggest increasing the size argument to JS_NewRuntime

[Adam Thompson]

On Fri, Feb 07, 2014 at 05:13:12AM -0800, Chris Brannon wrote:
> Karl Dahlke <eklhad@comcast.net> writes:
> 
> > Or maybe js returns 0, like any malloc failure,
> > and I don't watch for that and then I segfault.
> 
> Yes, this is correct.  My wording was loose.  JS returns NULL somewhere
> because of an out of memory condition, and the program eventually
> segfaults because that NULL is passed along to a function that cannot
> deal with it.
> As you say, we need lots more error legs.  I think we've had that
> discussion, and no one has written them yet.
> So I'll get started on those today.

Sorry, that was probably something I should've done.

> > But that doesn't explain why Chris simple program segfaults on debian js.
> 
> Yes, that's a different issue entirely. My program on Debian doesn't 
> even make it past JS initialization.  It fails before it can even get a
> JS context.

Interestingly a new mozjs24d package hit the repo at some stage last night or this morning.
I upgraded, built edbrowse and it still breaks in exactly the same place.

> > My wrappers around malloc do indeed print and exit upon malloc failure,
> > which probably saves me about 3,000 lines of error leg programming,
> 
> No, when Spidermonkey runs out of memory, the "out of memory" message is
> printed via my_ErrorReporter() from jsdom.cpp.
> E.G., try browsing http://the-brannons.com/array.html with db2.

This prints the error, but doesn't segfault.
This suggests it's killing js due to the unhandled exception.
ON my machine it gets to around 55100 elements before it stops with an error,
but doesn't segfault.

I wonder if you add an exception handler which handles the error and continues
if this'd cause the issue.

> We could write another set of wrappers for JS functions.  E.G.,
> our_JS_NewObject, which causes an exit when JS_NewObject fails.

I need to double-check the docs to see what the failure return is.
If it's throwing an exception rather than returning on out of memory then I
wonder what condition causes it to return NULL.

Cheers,
Adam.

[Edbrowse-dev] suggest increasing the size argument to JS_NewRuntime

[Chris Brannon]

>From what I can tell, the remaining segfaults all seem to be caused by
out-of-memory errors in JS.  As an aside, with debug level 2 or greater,
edbrowse will print "out of memory" before it crashes.
I've increased the size argument to JS_NewRuntime locally, and it
helps.  I'd suggest bumping it from 4 megabytes to at least 16 or 32.
Any objections?

Also, I pushed the patch that we discussed earlier today.

-- Chris

Re: [Edbrowse-dev] suggest increasing the size argument to JS_NewRuntime

[Adam Thompson]

On Thu, Feb 06, 2014 at 04:27:51PM -0800, Chris Brannon wrote:
> From what I can tell, the remaining segfaults all seem to be caused by
> out-of-memory errors in JS.  As an aside, with debug level 2 or greater,
> edbrowse will print "out of memory" before it crashes.
> I've increased the size argument to JS_NewRuntime locally, and it
> helps.  I'd suggest bumping it from 4 megabytes to at least 16 or 32.
> Any objections?

Not really, should this be configurable?

> Also, I pushed the patch that we discussed earlier today.

Ok, will build and test today.

Cheers,
Adam.