Closed
Bug 1056418
Opened 10 years ago
Closed 10 years ago
testGCFinalizeCallback failure with no default compartment object on the context
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: bholley, Assigned: jonco)
References
Details
Attachments
(1 file)
3.59 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
This seems to be the only remaining failure blocking my patches in bug 981218 to remove default compartment objects (which would be super-duper-awesome). I discussed it with terrence and bill, and they suggested that I file a bug and NI jonco.
Jonco, is there any chance you can look at this before thursday afternoon? I'm leaving on PTO tomorrow afternoon (thursday) and it would be awesome to get bug 981218 landed before I leave (and thus for this cycle). No worries if not though - it can probably wait until the next cycle.
To reproduce, apply this patch:
diff --git a/js/src/jsapi-tests/tests.h b/js/src/jsapi-tests/tests.h
index dfe0023..004c85c 100644
--- a/js/src/jsapi-tests/tests.h
+++ b/js/src/jsapi-tests/tests.h
@@ -306,6 +306,7 @@ class JSAPITest
if (!cx)
return nullptr;
JS_SetErrorReporter(cx, &reportError);
+ JS::ContextOptionsRef(cx).setNoDefaultCompartmentObject(true);
return cx;
}
And run jsapi-tests testGCFinalize. Output:
TEST-UNEXPECTED-FAIL | testGCFinalizeCallback | CHECK failed: FinalizeCalls % 2 == 1CHECK failed: checkMultipleGroups()
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(jcoppeard)
Assignee | ||
Comment 1•10 years ago
|
||
It took me a while to see what was happening but it seems it's just the test code at fault here.
Calling JSAPITest::createGlobal() did more than I expected and overwrote the objects main global root with the new object, unrooting the original global. This can prompt the GC run a second collection to clean up the compartment which it thinks should die as it has no roots or incoming pointers to it, and this in turn confuses the test by calling the finalize callback more than the expected number of times.
The fix is just to not overwrite JSAPITest::global when creating new globals inside the test.
I also factored out add/remove of the finalize callback into init()/uninit() overrides so that if the test fails then the callback is removed.
Assignee: nobody → jcoppeard
Attachment #8476634 -
Flags: review?(terrence)
Flags: needinfo?(jcoppeard)
Comment 2•10 years ago
|
||
Comment on attachment 8476634 [details] [diff] [review]
finalize-callback-patch
Review of attachment 8476634 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/jsapi-tests/testGCFinalizeCallback.cpp
@@ +37,5 @@
> CHECK(global1);
> CHECK(global2);
> CHECK(global3);
>
> +
Extra \n?
Attachment #8476634 -
Flags: review?(terrence) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in
before you can comment on or make changes to this bug.
Description
•