1078153 - Callback object passed to SpecialPowers.exactGC becomes dead before it is called back

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[JW Wang [:jwwang] [:jw_wang]]

Found this error when running mochitests.

https://tbpl.mozilla.org/php/getParsedLog.php?id=49611644&tree=Try&full=1
https://hg.mozilla.org/try/file/e6f20f25d904/testing/specialpowers/content/specialpowersAPI.js#l1392

00:12:18     INFO -  exactGC#1 callback=function () {
00:12:18     INFO -        if (callback) {
00:12:18     INFO -          callback();
00:12:18     INFO -        }
00:12:18     INFO -      }
00:12:18     INFO -  exactGC#2 callback=function () {
00:12:18     INFO -        if (callback) {
00:12:18     INFO -          callback();
00:12:18     INFO -        }
00:12:18     INFO -      }
00:12:18     INFO -  exactGC#3 callback=function () {
00:12:18     INFO -        if (callback) {
00:12:18     INFO -          callback();
00:12:18     INFO -        }
00:12:18     INFO -      }
00:12:18     INFO -  exactGC#2 callback=function () {
00:12:18     INFO -        if (callback) {
00:12:18     INFO -          callback();
00:12:18     INFO -        }
00:12:18     INFO -      }
00:12:18     INFO -  JavaScript error: chrome://specialpowers/content/specialpowersAPI.js, line 1404: TypeError: can't access dead object

It looks like the callback object is GCed during exactGC() and not called back to the called. I am not sure if this is a bug or wrong usage of javascript.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Hi Kyle,
Do you have any idea about this error? Do we capture the callback object in a wrong way?

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This can happen if the callback object is created inside a window that is then navigated away before the callback runs.  exactGC happens asynchronously, so if that happens by the time we run the callback we won't be able to call it anymore.

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

The callback object is created at [1], which is a local variable in a function. Somehow, the user of SpecialPowers.exactGC will be surprised that the callback will not be called back under some conditions. We should rewrite SpecialPowers.exactGC in a way where the callback won't get lost.


[1] http://hg.mozilla.org/mozilla-central/file/4534f97c4633/content/media/test/manifest.js#l848

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1078153_fix_SpecialPowers.exactGC.patch

capture the callback in a generated function object to prevent it becomes dead during GC.

Try: https://tbpl.mozilla.org/?tree=Try&rev=6e312e99dbb5
Most green.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8511853 [details] [diff] [review]
1078153_fix_SpecialPowers.exactGC.patch

Review of attachment 8511853 [details] [diff] [review]:
-----------------------------------------------------------------

Are you sure this works?  I don't see any reason why this should behave differently than what is currently there, although that may be because I'm reviewing this at the end of a long day.

The problem fundamentally is that exactGC is asynchronous, and it's racing with the unloading of the mochitest, right?

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

It works for the test cases and I don't see the error messages anymore.

The callback object is captured by |scheduledGCCallback| which is passed to Cu.schedulePreciseGC(). The callback object shouldn't be eligible for GC as long as it is captured by the function object. I don't know why this fails to work as intended. Therefore, I tried to capture the callback object in different ways and finally found some one that works.

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

http://hg.mozilla.org/mozilla-central/file/675913ddbb55/dom/media/test/manifest.js#l843

The test runner will not call SimpleTest.finish() to finish the test until GC callback is received. So the window object should be still alive until the GC callback to finish the test.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8511853 [details] [diff] [review]
1078153_fix_SpecialPowers.exactGC.patch

Review of attachment 8511853 [details] [diff] [review]:
-----------------------------------------------------------------

Based on comment 7 I don't really understand why this works and the current code does not, but ok.  This certainly shouldn't hurt anything.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cfed42e80971

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cfed42e80971