Closed
Bug 155192
Opened 22 years ago
Closed 21 years ago
crash [@nsRDFResource::~nsRDFResource] if gRDFService wasn't created
Categories
(Core Graveyard :: RDF, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: timeless, Assigned: timeless)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file, 3 obsolete files)
1.79 KB,
patch
|
tingley
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
run xpcshell
enter:
for (a in Components.classes)
try
{
b=Components.classes[a].createInstance(Components.interfaces.nsICollection);
if (b)
dump (a)
}
catch(e){}
Expected results: not much, perhaps nothing.
Actual results: crash
(stack from cvs build from last week, w32, opt profile)
nsRDFResource::~nsRDFResource(nsRDFResource * const 0x01258f90) line 71 + 7
bytes
nsMsgFolder::~nsMsgFolder(nsMsgFolder * const 0x01258f90) line 223 + 51 bytes
nsImapMailFolder::`scalar deleting destructor'(nsImapMailFolder * const
0x01258f90, unsigned int 1) + 8 bytes
nsRDFResource::Release(nsRDFResource * const 0x01258f90) line 83 + 32 bytes
nsImapMailFolderConstructor(nsISupports * 0x00000000, const nsID & {...}, void
* * 0x0012e9c4) line 59 + 86 bytes
nsGenericFactory::CreateInstance(nsGenericFactory * const 0x01258df8,
nsISupports * 0x00000000, const nsID & {...}, void * * 0x0012e9c4) line 75 + 14
bytes
nsComponentManagerImpl::CreateInstance(nsComponentManagerImpl * const
0x0038b768, const nsID & {...}, nsISupports * 0x00000000, const nsID & {...},
void * * 0x01258df8) line 1816 + 16 bytes
nsComponentManager::CreateInstance(const nsID & {...}, nsISupports *
0x00000000, const nsID & {...}, void * * 0x0012e9c4) line 103
nsJSCID::CreateInstance(nsJSCID * const 0x00b4ebb0, nsISupports * * 0x0012ea04)
line 796
XPTC_InvokeByIndex(nsISupports * 0x00b4ebb0, unsigned int 10, unsigned int 1,
nsXPTCVariant * 0x0012ea04) line 106
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
-1845493760) line 1994 + 22 bytes
XPC_WN_CallMethod(JSContext * 0x00af4bd0, JSObject * 0x00a7df38, unsigned int
1, long * 0x00acac30, long * 0x00acabd0) line 1266 + 10 bytes
js_Invoke(JSContext * 0x00000001, unsigned int 1, unsigned int 0) line 788 + 17
bytes
js_Interpret(JSContext * 0x00af4bd0, long * 0x0012feb8) line 2744
js_Execute(JSContext * 0x00a7d4b8, JSObject * 0x00a7d4b8, JSScript *
0x00b11ed0, JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012feb8) line
970
JS_ExecuteScript(JSContext * 0x00af4bd0, JSObject * 0x00a7d4b8, JSScript *
0x00b11ed0, long * 0x0012feb8) line 3274 + 26 bytes
Process(JSContext * 0x00af4bd0, JSObject * 0x00a7d4b8, char * 0x00000011,
_iobuf * 0x7803bb48) line 517 + 15 bytes
ProcessArgs(JSContext * 0x00af4bd0, JSObject * 0x00a7d4b8, char * * 0x003841d4,
int 0) line 655 + 28 bytes
main(int 1, char * * 0x003841d0) line 938
XPCSHELL! mainCRTStartup + 227 bytes
KERNEL32! 77e8d326()
- this 0x01258f90
|+ nsIRDFResource {...}
| mRefCnt 1
|+ gRDFService 0x00000000
| gRDFServiceRefCnt 0
|+ mURI 0x00000000 ""
\+ mDelegates 0x00000000
gRDFService->UnregisterResource(this);
I'm not certain my steps to reproduce were quite right.
here's a real log:
F:\build\mozilla\js\src>\build\mozilla\dist\WIN32_O.OBJ\bin\xpcshell.exe
js> for (a in Components.classes)
try {
b=Components.classes[a].createInstance(Components.interfaces.nsICollection);
if (b) dump (a)
}
finally{}
uncaught exception: [Exception... "Component returned failure code: 0x80570015
(NS_ERROR_XPC_CI_RETURNED_FAILURE) [nsIJS
CID.createInstance]" nsresult: "0x80570015 (NS_ERROR_XPC_CI_RETURNED_FAILURE)"
location: "JS frame :: typein :: <TOP_L
EVEL> :: line 8" data: no]
js> for (a in Components.classes)
try {
b=Components.classes[a].createInstance(Components.interfaces.nsICollection);
if (b) dump (a)
}
catch(e){}
<crash>
Comment 2•22 years ago
|
||
That patch doesn't make sense to me -- it looks like it just leaks all the resources.
Even with |if (!gRDFService) return; |, it's still just wallpaper. The resource reference count in the service (which presumably went to 0 early, triggering destruction) and the actual number of extant resources really shouldn't be out of synch.
Comment 3•22 years ago
|
||
Erm, I only just realized that the bug summary includes "if gRDFService wasn't created". Curse you, lynx.
So it's not a consistency issue after all. In that case, a wallpaper fix may be appropriate. (But the patch still needs logical inversion -- only return if the service doesn't exist.)
Comment on attachment 89787 [details] [diff] [review]
backwards logic
doh! i can't believe i typed that. ... ...
Attachment #89787 -
Attachment description: handle strange case → backwards logic
Attachment #89787 -
Flags: needs-work+
Attachment #89787 -
Attachment is obsolete: true
Comment 7•22 years ago
|
||
So, my question is: "how can an RDF resource be created without the RDF service
being around?"
timeless: can you put a breakpoint in the nsRDFResource ctor and see what's
happening?
Comment 8•22 years ago
|
||
I was wondering that too.
I think the story is that the dummy resource the js snippet creates doesn't ever get Init()'d. Since Init() adds to the gRDFServiceRefCount in nsRDFResource.cpp (which triggers service creation), we end up with a resource that isn't counted in the gRDFServiceRefCnt.
So it's sort of a refcount imbalance after all, since we have a mix of real resources created for chrome, etc, and at least one dummy.
That said, my build is behaving a bit funky as I try to play with this.
Comment 9•22 years ago
|
||
Ok, I swear I'm going to quit posting in lynx. Seriously.
Comment 10•22 years ago
|
||
I'd be tempted to try something like this instead. In order for the destructor
to behave correctly, we want to make sure that registered resources get cleaned
up properly, and that unregistered resources are more or less ignored. The
existence of a non-null |mURL| can serve as a proxy for an "isRegistered" flag,
since it is only allocated during Init(), when registering and
gRDFService-refcounting also takes place.
Thoughts?
Attachment #98044 -
Flags: superreview?(rjc)
Attachment #98044 -
Flags: review+
Attachment #98044 -
Flags: superreview?(rjc) → superreview?(bzbarsky)
Updated•22 years ago
|
Attachment #98044 -
Flags: superreview?(bzbarsky) → superreview+
Comment 11•22 years ago
|
||
11/17/2002 20:44 timeless%mozdev.org mozilla/ rdf/ util/ src/ nsRDFResource.cpp
1.38 9/7 Bug 155192 crash [@nsRDFResource::~nsRDFResource] if gRDFService
wasn't created
patch by tingley@sundell.net r=timeless sr=bz
this has been checked in, marking fixed.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•21 years ago
|
||
*** Bug 217461 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 14•21 years ago
|
||
chase: per bug 217461 your patch didn't work.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 15•21 years ago
|
||
The resource should be holding a reference to the rdfservice, which
should prevent this from happening in regular usage. Since it's
clearly happening anyway, I'm ok with letting timeless plug the
hole -- but I think there's some bad stuff going on in mailnews as
well that I'd like to track down, if possible.
Comment 16•21 years ago
|
||
cc'ing Axel for his enjoyment, since I have a hunch some user of
resource factories may be involved in this.
We should go ahead and patch the crash anyways (I'm torn on adding
an assertion, since I don't think the resource that triggers the
crash is necessarily the guilty one).
I'm wondering if we need to be more defensive with regard to the
possibility of external nsIRDFResource implementations. Currently,
any such implementation that doesn't hold a per-resource reference
to the RDFService would cause the same problem.
Assignee | ||
Comment 17•21 years ago
|
||
Ok, this should plug the whole in the original way, return early.
it also makes the alloc/free consistent and switches to CallGetService.
none of the changes to :Init are necessary for this bug.
Attachment #89938 -
Attachment is obsolete: true
Attachment #98044 -
Attachment is obsolete: true
Attachment #130585 -
Flags: superreview?(bz-vacation)
Attachment #130585 -
Flags: review?(tingley)
Updated•21 years ago
|
Attachment #130585 -
Flags: superreview?(bz-vacation) → superreview+
Updated•21 years ago
|
Attachment #130585 -
Flags: review?(tingley) → review+
Assignee | ||
Comment 18•21 years ago
|
||
checked in. people can use the other bug to investigate the mailnews abuse
Status: REOPENED → RESOLVED
Closed: 22 years ago → 21 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Crash Signature: [@nsRDFResource::~nsRDFResource]
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•