58252 - Crash [@ RDFServiceImpl::GetDataSource]

Status: VERIFIED FIXED

(Core Graveyard :: RDF, defect, P3)

Description

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This is a topcrash reported by Talkback. There are 14 instances according to the
Talkback sidebar. One Linux, other various Windows versions. The crash seems to
happen mostly in startup. I cannot reproduce this.

It looks like the crash happens because for some reason we are handed either a
null or garbage pointer as the out param pointer and when we assign to it we crash.

If we are handed a null pointer it would be easy to prevent a crash here. Right
now the method is not checking if the out param != 0, which is the Right Thing
To Do(TM). This is more like band-aid than real fix, we should also find out WHY
someone is passing in a null pointer (if this is the cause of the crash).

Here is the stack trace:

RDFServiceImpl::GetDataSource
[d:\builds\seamonkey\mozilla\rdf\base\src\nsRDFService.cpp line 1136]
XPTC_InvokeByIndex
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp
line 139]
nsXPCWrappedNativeClass::CallWrappedMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativeclass.cpp line
915]
WrappedNative_CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp line
223]
js_Invoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c line 822]
js_Interpret [d:\builds\seamonkey\mozilla\js\src\jsinterp.c line 2621]
js_Execute [d:\builds\seamonkey\mozilla\js\src\jsinterp.c line 994]
JS_ExecuteScript [d:\builds\seamonkey\mozilla\js\src\jsapi.c line 3044]
nsJSContext::ExecuteScript
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp line 726]
nsXULDocument::ExecuteScript
[d:\builds\seamonkey\mozilla\rdf\content\src\nsXULDocument.cpp line 5488]
nsXULDocument::OnStreamComplete
[d:\builds\seamonkey\mozilla\rdf\content\src\nsXULDocument.cpp line 5438]
nsStreamLoader::OnStopRequest
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamLoader.cpp line 123]
nsJARChannel::OnStopRequest
[d:\builds\seamonkey\mozilla\netwerk\protocol\jar\src\nsJARChannel.cpp line 703]
nsOnStopRequestEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsAsyncStreamListener.cpp line 302]
nsStreamListenerEvent::HandlePLEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsAsyncStreamListener.cpp line 106]
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c line 581]
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c
line 517]
_md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c line
1051]
KERNEL32.DLL + 0x248f7 (0xbff848f7)
0x0068820e
0x00058f64

Comment 1

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Potential fix: Check that out param is not null

Comment 2

[Chris Waterson]

I'm 99% sure that the crash is on the addref at line 1135, not the assign.
XPConnect is almost certainly *not* giving us bad return slots, or we'd be
crashing all over. (Furthermore, this "off-by-one" in Win32 crash reports is
fairly typical; the PC is set to the instruction immediately after the branch
through a garbage vtable.)

Anyway, see bug 57764, where I tried to fix a known problem. I'll leave this bug
open, because I haven't looked at builds post 2000-10-24, when this fix was
checked in.

Comment 3

[Chris Waterson]

jband, you think you're giving me a bad pointer for an retval? See

http://www.mozilla.org/projects/seamonkey/reports/ns6analysis.html#RDFServiceImpl::GetDataSource

Comment 4

[Chris Waterson]

Yeah, looking at

http://cyclone/reports/incidenttemplate.cfm?setvar=DeveloperDeveloperTabSet:Code%20Around%20the%20PC&bbid=19403673#DeveloperMachineState

you can tell that it's blowing up on the virtual call to AddRef(). So, as I
suspected, something's not being removed from the datasource table. Bugger.

Comment 5

[Chris Waterson]

Attachment: two more places where native datasources failed to UnregisterDataSource(), plus patch to make RDFServiceImpl support weak references

Comment 6

[Chris Waterson]

rjc: I found two more datasources that fail to match calls to
RegisterDataSource() with calls to UnregisterDataSource(), nsLocalStore.cpp and
nsInternetSearchService.cpp. Could you r= the changes?

I made it so that the RDF service could support weak references (and modernized
some of the macro-munging in that ancient file); scc, would you mind sanity
checking that code?

Comment 7

[Robert John Churchill]

r=rjc for the UnregisterDataSource() changes

Comment 8

[Alec Flett]

hooray, thanks for fixing this - sr=alecf

Comment 9

[Heikki Toivonen (remove -bugzilla when emailing directly)]

I am nominating this for RTM because this is a topcrash. I am also adding rtm+
on the status whisteboard because this has been properly reviewed and super
reviewed. Hopefully we could squeeze this one in...

Comment 10

[selmer (gone)]

This bug is in candidate limbo.  We will reconsider this fix once we have a 
candidate in hand, but we can't take this fix before then.  Please check into 
the trunk ASAP.

Comment 11

[Chris Waterson]

Fixed on trunk.

Comment 12

[selmer (gone)]

rtm++, please checkin ASAP so we can build today.

Comment 13

[Chris Waterson]

Fix checked in on branch.

Comment 14

[lchiang]

Between 2000102600 and today , there have been 9 incidents per Talkback of this
crash. This fix was done on 11/1.

I looked all incidents where Call Stack Signature contains
RDFServiceImpl::GetDataSource. Since 11/1 at 4:26pm, all reported incidients
with this stack track are from PR3.   Prior to that, there were many incidents
from folks using the latest builds.

I think we can mark this verified.