Closed
Bug 67258
Opened 24 years ago
Closed 24 years ago
Components.results sometimes has no properties
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
VERIFIED
FIXED
People
(Reporter: jband_mozilla, Assigned: jband_mozilla)
Details
Attachments
(2 files)
2.88 KB,
patch
|
Details | Diff | Splinter Review | |
5.15 KB,
patch
|
Details | Diff | Splinter Review |
nsXPCComponents_Results stores state about the wrapper around itself. That state can be wrong if the wrapper is gc'd and a new wrapper created in its place. dmose tripped over this bug in his twisted ldap work. Commented patch to follow.
Assignee | ||
Comment 1•24 years ago
|
||
Comment 2•24 years ago
|
||
I pulled the tip version of xpccomponents.cpp, applied this patch, and the problem seems fixed. Thanks John!
Comment 3•24 years ago
|
||
r/sr=brendan@mozilla.org, but dmose's code is still twisted :-) /be
Comment 4•24 years ago
|
||
http://lxr.mozilla.org/seamonkey/search?string=mCacheFilled nsJSIID seems to use the same pattern, could it (in any way) be suffering from the same problem as nsXPCComponents_Results?
Assignee | ||
Comment 5•24 years ago
|
||
jag: It turns out not to matter for the nsJSIID objects. They represent the "nsIFoo" in "Components.interfaces.nsIFoo" and the properties on them that we are adding represent the 'const' members declared in idl. The nsJSIID objects themselves are rooted because their wrappers' JSObjects are added as properties of the 'interfaces' JSObject. In the case where the 'interfaces' JSObject itself is collected then we would also end up lazily recreating the nsJSIID objects anyway if and when a new 'interfaces' object where created. So, we never hit a case where the properties of the nsJSIID JSObject go away without the nsJSIID C++ object itself also going away. An xpcshell test case for both of these is: print(Components.results.NS_OK); print(Components.interfaces.nsICategoryManager.FALLBACK); gc(); print(Components.results.NS_OK); print(Components.interfaces.nsICategoryManager.FALLBACK); This should print something like: 0 1 before 6309, after 5598, break 00000000 0 1 Without the patch I get: 0 1 before 6309, after 5598, break 00000000 undefined 1 Still, it is perfectly reasonable to apply the same (obj != mObj) test in nsJSIID. There is certainly nothing wrong with this safer pattern and it might save us if the system changes a bit in the future. I have to admit that I failed to think of this case and only discovered that it was not a problem by testing it and *then* thinking through why. Thanks! John.
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•24 years ago
|
||
Comment 7•24 years ago
|
||
Sure, good patterns should spread, replacing less good ones. r/sr=brendan@mozilla.org. /be
Comment 8•24 years ago
|
||
Well, I understand the changes you've made, it seems you've correctly changed all related code, and I'll just take your word for it that |obj| is the right one :-) r=jag, fwiw.
Assignee | ||
Comment 9•24 years ago
|
||
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•