From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <chris@the-brannons.com>
Received: from localhost (unknown [IPv6:2602:47:d673:8e00:12bf:48ff:fe7c:5584])
 by hurricane.the-brannons.com (Postfix) with ESMTPSA id EF5FB78495
 for <Edbrowse-dev@lists.the-brannons.com>;
 Wed, 19 Feb 2014 09:35:13 -0800 (PST)
From: Chris Brannon <chris@the-brannons.com>
To: Edbrowse-dev@lists.the-brannons.com
References: <20140119042137.eklhad@comcast.net>
 <20140219113807.GJ28870@toaster.adamthompson.me.uk>
 <20140219141237.GK28870@toaster.adamthompson.me.uk>
 <87lhx76q7z.fsf@mushroom.PK5001Z>
 <20140219165245.GL28870@toaster.adamthompson.me.uk>
Date: Wed, 19 Feb 2014 09:34:22 -0800
Message-ID: <877g8r6m75.fsf@mushroom.PK5001Z>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Edbrowse-dev] segfault with small pool
X-BeenThere: edbrowse-dev@lists.the-brannons.com
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: Edbrowse Development List <edbrowse-dev.lists.the-brannons.com>
List-Unsubscribe: <http://lists.the-brannons.com/mailman/options/edbrowse-dev>, 
 <mailto:edbrowse-dev-request@lists.the-brannons.com?subject=unsubscribe>
List-Archive: <http://lists.the-brannons.com/mailman/private/edbrowse-dev/>
List-Post: <mailto:edbrowse-dev@lists.the-brannons.com>
List-Help: <mailto:edbrowse-dev-request@lists.the-brannons.com?subject=help>
List-Subscribe: <http://lists.the-brannons.com/mailman/listinfo/edbrowse-dev>, 
 <mailto:edbrowse-dev-request@lists.the-brannons.com?subject=subscribe>
X-List-Received-Date: Wed, 19 Feb 2014 17:35:14 -0000

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 */