155192 - crash [@nsRDFResource::~nsRDFResource] if gRDFService wasn't created

Status: RESOLVED FIXED

(Core Graveyard :: RDF, defect)

Description

[timeless]

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);

Comment 1

[timeless]

Attachment: backwards logic

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

[Chase Tingley]

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

[Chase Tingley]

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 4

[timeless]

Comment on attachment 89787 [details] [diff] [review]
backwards logic

doh! i can't believe i typed that. ... ...

Comment 5

[Chris Waterson]

-> timeless

Comment 6

[timeless]

Attachment: corrected patch

Comment 7

[Chris Waterson]

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

[Chase Tingley]

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

[Chase Tingley]

Ok, I swear I'm going to quit posting in lynx.  Seriously.

Comment 10

[Chase Tingley]

Attachment: another approach

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?

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

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 [e-mail] r=timeless sr=bz

this has been checked in, marking fixed.

Comment 12

[benc]

tever is not RDF QA anymore

Comment 13

[timeless]

*** Bug 217461 has been marked as a duplicate of this bug. ***

Comment 14

[timeless]

chase: per bug 217461 your patch didn't work.

Comment 15

[Chase Tingley]

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

[Chase Tingley]

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.

Comment 17

[timeless]

Attachment: -uwp plug hole

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.

Comment 18

[timeless]

checked in. people can use the other bug to investigate the mailnews abuse