590284 - test-memory.testMemory failure on Mac OS X

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

On Mac OS X with the latest trunk nightly Firefox, test-memory.testMemory is failing:

error: TEST FAILED: test-memory.testMemory (failure)
error: fail: ({}) != null
info: Traceback (most recent call last):
  File "resource://jetpack-core-jetpack-core-lib/timer.js", line 58, in notify
    this._callback.apply(null, []);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 259, in anonymous
    timer.setTimeout(function() { onDone(self); }, 0);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 284, in runNextTest
    self.start({test: test, onDone: runNextTest});
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 302, in start
    this.test.testFunction(this);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 57, in runTest
    test(runner);
  File "resource://jetpack-core-jetpack-core-tests/test-memory.js", line 10, in anonymous
    test.assertEqual(objs[0].weakref.get(), null);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 231, in assertEqual
    this.fail(message);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 149, in fail
    console.trace();

Comment 1

[Atul Varma [:atul]]

I'm able to reproduce this bug only on the most recent nightly--a nightly I had from a week ago doesn't exhibit this error, and since the memory tracking code in jetpack hasn't changed at all in months and the test that's failing was written to make sure that garbage collection works properly, I'm concerned that this bug might actually be one in the platform whereby weak references aren't working for some reason... Hmm.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

If the problem is a platform bug, what do we do about the test?  Should we disable it until the platform bug is fixed?  Or should we ship with a failing test?

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: works around problem

Here's a patch that works around the problem.  It comments out the test until the underlying platform problem is resolved.  In conversations with Atul, we agreed that this seems like the best solution for the moment.

Atul is also looking into a reduced test case, which we'll then attach to a platform bug report, which we'll then reference in this bug report so folks who see the comment in the test can follow it to this bug report and thence to the underlying platform bug report.

Comment 4

[Atul Varma [:atul]]

Here's a simple test to determine whether the platform bug is present or not:

exports.testGeckoGC = function testGeckoGC(test) {
  var {Cu} = require("chrome");
  var obj = {};
  var weak = Cu.getWeakReference(obj);

  obj = null;
  Cu.forceGC();
  test.assertEqual(weak.get(), null);
};

It's hard to imagine a test like this not already being in mozilla-central, though... In any case, it indeed works on an older nightly, but not on the most recent one. Seems like it could be used as a litmus to decide whether or not to run the one that's failing in this bug, too, actually.

Comment 5

[Atul Varma [:atul]]

Myk pointed me at this, which appears to test the same thing:

  http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/tests/unit/test_js_weak_references.js

So the nightly must have been failing that test...

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 468858 [details] [diff] [review]
patch v1: works around problem

Update: after further discussion, I think it makes more sense to leave the test enabled, since the problem seems like a regression that will get resolved quickly, if it hasn't been resolved already, given that there is a test for it in a platform test suite.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Note: I checked with Atul via IM, and he agrees with the sentiment in comment 6.  Resolving WONTFIX and removing blocker status.

Comment 8

[Drew Willcoxon :adw]

Weird, but looking over the mozilla-central pushlog going back the past couple of days, I don't see any GC-related commits, backouts, or test fixes...  Consistent test failures are either fixed or backed out on the spot.

http://hg.mozilla.org/mozilla-central/pushloghtml

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Yeah, I'm going to reopen this bug, since it's still a problem.  I'm not going to block 0.7 on it, given the uncertainty over its provenance, but we should figure out whether this is really a platform bug (that happens not to be caught by the platform test) or somehow a Jetpack issue.

Comment 10

[Atul Varma [:atul]]

Yeah, this is still occurring on the latest nightlies. :(

Because the reliability of the entire memory debugging infrastructure of Jetpack is put in question by bug 592180, at the very least, I'm going to disable all of it by default and only re-enable it once that bug is fixed.

Comment 11

[Atul Varma [:atul]]

I've just filed bug 592774 to track this and make sure we re-enable this test once bug 592180 is fixed.

Comment 12

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".

Comment 13

[Myk Melez [:myk] [@mykmelez]]

The test now passes when reenabled; perhaps the problem was in platform code that has been fixed.