482179 - JS args not properly QueryInterface'd sometimes (wrappers get GC'd too easily?)

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

[Ed Lee :Mardak]

I'm getting a very strange behavior with Weave when switching things to use a lazy service getter.

The relevant patch change is as follows:
-    let ios = Cc["@mozilla.org/network/io-service;1"].
-      getService(Ci.nsIIOService);
-    this._lastChannel = ios.newChannel(this.spec, null, null).
+    this._lastChannel = Svc.IO.newChannel(this.spec, null, null).
       QueryInterface(Ci.nsIRequest);

But the problem isn't here -- to work around the problem I had to do this:
 ChannelListener.prototype = {
   onStartRequest: function Channel_onStartRequest(channel) {
+    channel.QueryInterface(Ci.nsIHttpChannel);
     this._log.debug(channel.requestMethod + " request for " +
       channel.URI.spec);

If I print out what "channel" is before and after the added QueryInterface, they're as follows:

original (Cc[].getService) before/after QI:
[xpconnect wrapped (nsISupports, nsIChannel, nsIRequest, nsIHttpChannel)]
[xpconnect wrapped (nsISupports, nsIChannel, nsIRequest, nsIHttpChannel)]

new (Svc.IO) before/after QI:
[xpconnect wrapped nsIRequest]
[xpconnect wrapped (nsISupports, nsIRequest, nsIHttpChannel)]

Only afterwards does Channel_onStartRequest stop complaining about channel.URI being undefined.

Svc.IO is a lazy service getter that deletes the getter and assigns then returns the service.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Are you sure the lazy getter is the only relevant change you're making? nsIHttpChannel objects don't implement classinfo, so I would expect you to always need the QI. Was the object implicitly being QIed some other way before this change?

Having a working testcase would help debugging immensely.

Comment 2

[Ed Lee :Mardak]

Neil says "that's just your xpconnect wrapper getting GC'd and you need to call QI again"

I've reduced the testcase to 2 modules: util.js and resource.js.

util.js just exports a single object with a single attribute for the IOService (non-lazy). It's so short, here it is:

const EXPORTED_SYMBOLS = ['Svc'];
let Svc = {
  IO: Components.classes["@mozilla.org/network/io-service;1"].
      getService(Components.interfaces.nsIIOService)
};

resource.js just imports util.js and exports Resource that asyncOpens a uri with either Svc.IO or Cc[]:

const EXPORTED_SYMBOLS = ['Resource'];
const Cc = Components.classes;
const Ci = Components.interfaces;
const Cu = Components.utils;
Cu.import("resource://badext/util.js");
function Resource(b) {
  (b ? Svc.IO : Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService)).
  newChannel(Svc.IO.newURI("http://ed.agadak.net/", null, null).spec, null, null).
  asyncOpen({
    onStartRequest: function(c) Cu.reportError(c),
    onStopRequest: function() {},
    onDataAvailable: function() {}
  }, null);
}

If you install the attached add-on, you can open the error console and put in:
Components.utils.import("resource://badext/resource.js"); [new Resource(), new Resource(1)]

It'll immediately return [object Object],[object Object] and eventually..
Error: [xpconnect wrapped (nsISupports, nsIChannel, nsIRequest)]
Error: [xpconnect wrapped nsIRequest]

Where the first "error" is the Cc[] case and the second "error" is Svc.IO.

Definitely sounds GCish as if I change the uri that it opens, it can make both Cc[] and Svc.IO result in a plain nsIRequest. And if I combine the code into a single file, both keep the wrappers.

Comment 3

[Ed Lee :Mardak]

Attachment: badext add-on: resource.js and util.js

Comment 4

[Ed Lee :Mardak]

I suppose "properly QI-d" isn't accurate, as the code isn't correctly QI-ing. So the correct "fix" is to always QI, but it's not what's expected? (at least not what the original Weave code was expecting ;))

Comment 5

[[e-mail]]

On that last point, I believe wrappers are only found in the scope of the global where they were created, so if you "see" the same object again in a new scope (as distinct to explicitly passing the wrapper) it will have a new wrapper.

Or maybe I should just shut up and let mrbkap explain all ;-)

Comment 6

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.