Re: [Edbrowse-dev] segfault with small pool

[Chris Brannon <chris@the-brannons.com>]

Adam Thompson <arthompson1990@gmail.com> writes:

> I'm not sure if that's even possible at the moment given the way some of the
> functions work. I'd hope that what happens when we destroy the context is that
> the context-linked roots also go away (which'd make sense).

They don't go away.  I just verified that with gdb.
I'm starting to see a possible fix, I think.
In javaSessionFail, do not call freeJavaContext at all.
Add a new variable to cw: cw->jss_saved.
Now javaSessionFail looks like this:
cw->jss_saved = cw->jss;
cw->jss = NULL;

Now we have two cases to consider.
1. Spidermonkey ran out of memory while parsing the page, or
2. Spidermonkey ran out of memory when handling a JS event like onclick.

For case 1, when we've finished parsing the page,
call freeJavaContext(cw->jss_saved) if cw->jss_saved is not NULL.
Set cw->jss_saved to NULL so we don't double-free.
I think we can do this at the end of parsePage.  There will be no
stack-allocated JS roots at this point.

It's going to be a lot messier for case 2, I think.  Can we just do the
"free-if-saved" check after each call to handlerGo?

BTW, freeJavaContext also has a bug.

void freeJavaContext(struct ebWindowJSState *state)
{
	if (state == NULL)
return;
if (state->jcx != NULL)
{
if (js::GetContextCompartment((const JSContext *) state->jcx) != NULL)
JS_LeaveCompartment(state->jcx, NULL);
		JS_DestroyContext(state->jcx);
}
		delete state;
}				/* freeJavaContext */

But state has HeapRooted objects after the context is destroyed.  Really
needs to be:

void freeJavaContext(struct ebWindowJSState *state)
{
	if (state == NULL)
		return;
	JSContext *cx = state->jcx;
	delete state;
	if (cx != NULL) {
		if (js::GetContextCompartment((const JSContext *) cx) != NULL)
			JS_LeaveCompartment(cx, NULL);
		JS_DestroyContext(cx);
}
}				/* freeJavaContext */