Re: [Edbrowse-dev] garbage collection1

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

[-- Attachment #1: Type: text/plain, Size: 615 bytes --]

Adam Thompson <arthompson1990@gmail.com> writes:

>> We should not be passing unrooted jsval as the fourth
>> argument to JS_DefineProperty as this can also lead to a crash.
>
> Yeah, I'm not sure how to work around this.

I am, and a patch is attached.  There may be other cases.  I can look through
the code for them.  I fixed the one I noticed.

> If you could test the latest code that'd be useful (corrected jwin and uo rooting).

I'm not seeing it.  All I see is that you merged the master branch back
into your repo.  Did you forget a push?

PS.  Thank you very much for all of your work on this!

-- Chris


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rootedval.diff --]
[-- Type: text/x-diff, Size: 1171 bytes --]

diff --git a/src/jsdom.cpp b/src/jsdom.cpp
index 7f3eef3..9c12bd7 100644
--- a/src/jsdom.cpp
+++ b/src/jsdom.cpp
@@ -1243,13 +1243,13 @@ Yeah, it makes my head spin too.
 	} else {
 	    v = JS_NewObject(jcx, cp, NULL, owner_root);
 	}
-	vv = OBJECT_TO_JSVAL(v);
+	JS::RootedValue rvv(jcx, OBJECT_TO_JSVAL(v));
 
 /* if no name, then use id as name */
 	if(!symname && idname) {
-	    JS_DefineProperty(jcx, owner_root, idname, vv, NULL, NULL, attr);
+	    JS_DefineProperty(jcx, owner_root, idname, rvv, NULL, NULL, attr);
 	} else if(symname && !dupname) {
-	    JS_DefineProperty(jcx, owner_root, symname, vv, NULL, NULL, attr);
+	    JS_DefineProperty(jcx, owner_root, symname, rvv, NULL, NULL, attr);
 	    if(stringEqual(symname, "action"))
 		establish_property_bool(v, "actioncrash", eb_true, eb_true);
 
@@ -1259,7 +1259,7 @@ Yeah, it makes my head spin too.
 	    establish_property_object(master, symname, v);
 	} else {
 /* tie this to something, to protect it from gc */
-	    JS_DefineProperty(jcx, owner_root, fakePropName(), vv,
+	    JS_DefineProperty(jcx, owner_root, fakePropName(), rvv,
 	       NULL, NULL, JSPROP_READONLY | JSPROP_PERMANENT);
 	}