662915 - Comparing XPCOM Objects fails under certain circumstances

Status: VERIFIED INVALID

(Core :: XPCOM, defect)

Description

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Testcase extension

This bug was found while looking for the culprit to bug 491118. Under certain circumstances, comparing xpcom instances fails. A testcase extension is attached. To use it, install in Firefox and then browse to <chrome://test-xpcsip/content/test-xpcsip.xul>. Afterwards, check your error console and make sure it says there are 0 observers left.

I've bisected this, but couldn't identify the exact changeset. Next, I tried mozilla-central nightly builds:


Last working Nightly:
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:2.0b2pre) Gecko/20100701
Minefield/4.0b2pre

First bad Nightly:
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:2.0b2pre) Gecko/20100702
Minefield/4.0b2pre 

about:buildconfig coming up soon.

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: about:buildconfig working (2010-07-01)

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: about:buildconfig failing (2010-07-02)

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

For reference, this is the result of my bisect. I couldn't get the exact changeset since not every revision worked, but at least its down to 32 changesets:

Last working revision:
changeset:   46996:fe832b373724
user:        Benjamin Smedberg <[e-mail]>
date:        Thu Jun 10 12:51:43 2010 -0400
summary:     Bug 570488 part B - fix and test loading XPT files from
JARs, needed for omnijar, r=mwu

First known bad revision:
changeset:   47029:014f02585f6d
user:        Benjamin Smedberg <[e-mail]>
date:        Tue Jun 22 20:21:34 2010 -0400
summary:     Rename NSGetModule to NSModule in various build scripts
for bug 568691.

Comment 4

[Daniel Holbert [:dholbert]]

Just to save others the trouble of looking it up -- those revisions are both encompassed in this push, towards the bottom:
http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=fe832b373724

(hgweb won't show a fromchange=foo&tochange=bar pushlog regression range if foo & bar are within the same push, AFAICT)

Comment 5

[Benjamin Smedberg]

Adding a debug dump to "compareObjects" gives:

COMPARING OBSERVERS: |[xpconnect wrapped nsIObserver @ 0xaaada68 (native @ 0xaacd498)]| |[xpconnect wrapped nsIObserver @ 0xaaadda8 (native @ 0xaacd5b0)]|
Source File: jar:file:///c:/builds/test-profile/extensions/[e-mail]!/components/testSip.js
Line: 140

This is a double-wrapped object: it's a nsIObserver implemented in JS, passed through XPConnect and back into JS. I'm not sure why we're ending up with different native addresses though: I don't think there ought to be any other wrappers involved.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Different native means we got a different XPCWrappedJS for it.

Comment 7

[Benjamin Smedberg]

Error: OBSERVER ADDED: |[xpconnect wrapped nsIObserver @ 0xab78e50 (native @ 0xabe5de8)]|, 1 total
Source File: jar:file:///c:/builds/test-profile/extensions/[e-mail]!/components/testSip.js
Line: 122

Error: OBSERVER REMOVED: |[xpconnect wrapped nsIObserver @ 0xab79128 (native @ 0xabe5ec8)]|, 1 remaining
Source File: jar:file:///c:/builds/test-profile/extensions/[e-mail]!/components/testSip.js
Line: 130

Rereading the testcase, I think this is a registration error. In testSip.js, it has:

    _xpcom_categories: [
      { category: "alarm-service-startup", service: true },
      { category: "alarm-service-shutdown", service: true }
    ],

Which was old-style registration. But in chrome.manifest, it says:

category alarm-service-startup calendar-alarm-monitor @mozilla.kewis.ch/testxpcsip/alarm-monitor;1
category alarm-service-shutdown calendar-alarm-monitor @mozilla.kewis.ch/testxpcsip/alarm-monitor;1

This contractID is not prefixed with "service,", which means that we're creating a new @mozilla.kewis.ch/testxpcsip/alarm-monitor;1 every time it calls into the appstartup notifier code. Fixing that registration error fixes the problem.

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Aaaarrrgh...so much testing and figuring out for such a simple issue! Thank you, I've tested it and it works great.