383116 - Extension data source isn't registered correctly and can crash

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[[e-mail]]

Comment 1

[[e-mail]]

Attachment: Proposed patch

Comment 2

[[e-mail]]

Oh, looks like I completely overlooked the steps to reproduce.
1. In the Error Console, evaluate
Components.classes['@mozilla.org/rdf/rdf-service;1'].getService(Components.interfaces.nsIRDFService).GetDataSource('rdf:extensions');
2. Open the Addons Manager (and possibly close it again for good measure)
3. Reevaluate the expression

Expected result: expression returns an [xpconnect wrapped nsIRDFDataSource]

Actual result at step 1: a factory not registered exception
Actual result at step 3: either a no such interface exception or a crash

Comment 3

[[e-mail]]

Comment on attachment 267132 [details] [diff] [review]
Proposed patch

Trying a different reviewer.

Comment 4

[Robert Strong (they/them - no direct email)]

Comment on attachment 267132 [details] [diff] [review]
Proposed patch

Neil, I should be able to get to this tonight. Thanks!

Comment 5

[Robert Strong (they/them - no direct email)]

Comment on attachment 267132 [details] [diff] [review]
Proposed patch

Neil, I'm still seeing a crash when using the steps to reproduce... no additional info beyong that as of yet.

Comment 6

[[e-mail]]

At what point are you crashing? Do you get any results from steps 1 or 3?

Comment 7

[Robert Strong (they/them - no direct email)]

Step 1 - I am using venkman since evaluate isn't working in the Error Console (see bug 385092)

Comment 8

[[e-mail]]

Odd, it doesn't crash for me there either, with the patch I just get
[xpconnect wrapped nsIRDFDataSource @ 0x41f2698 (native @ 0x409d538)]

Comment 9

[Robert Strong (they/them - no direct email)]

I'll see if I can track down what is going on

Comment 10

[Robert Strong (they/them - no direct email)]

I'm crashing at
http://lxr.mozilla.org/seamonkey/source/rdf/base/src/nsRDFService.cpp#1440

xul.dll!RDFServiceImpl::GetDataSource(const char * aURI=0x04e874d0, int aBlock=0, nsIRDFDataSource * * aDataSource=0x0012d320)  Line 1440 + 0x9 bytes	C++
xul.dll!RDFServiceImpl::GetDataSource(const char * aURI=0x04e874d0, nsIRDFDataSource * * aDataSource=0x0012d320)  Line 1403	C++
xul.dll!NS_InvokeByIndex_P(nsISupports * that=0x0000000f, unsigned int methodIndex=2, unsigned int paramCount=1233680, nsXPTCVariant * params=0x0012d1c8)  Line 102	C++
xul.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=15)  Line 2240 + 0x1d bytes	C++
xul.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=CALL_METHOD)  Line 2240 + 0x1d bytes	C++
xul.dll!XPC_WN_CallMethod(JSContext * cx=0x05c47800, JSObject * obj=0x083eeee0, unsigned int argc=1, long * argv=0x0771b60c, long * vp=0x0012d5ac)  Line 1467 + 0xe bytes	C++
js3250.dll!js_Invoke(JSContext * cx=0x05c47800, unsigned int argc=1, unsigned int flags=0)  Line 1306 + 0x20 bytes	C
js3250.dll!js_Interpret(JSContext * cx=0x05c47800, unsigned char * pc=0x06325306, long * result=0x0012dba4)  Line 3992 + 0xf bytes	C
js3250.dll!js_Execute(JSContext * cx=0x05c47800, JSObject * chain=0x058e8360, JSScript * script=0x063252b8, JSStackFrame * down=0x0012e2bc, unsigned int flags=32, long * result=0x0012dcc8)  Line 1567 + 0x13 bytes	C
js3250.dll!obj_eval(JSContext * cx=0x05c47800, JSObject * obj=0x058e8360, unsigned int argc=2, long * argv=0x0771b5ec, long * rval=0x0012dcc8)  Line 1394 + 0x1b bytes	C
js3250.dll!js_Invoke(JSContext * cx=0x05c47800, unsigned int argc=2, unsigned int flags=0)  Line 1306 + 0x20 bytes	C
js3250.dll!js_Interpret(JSContext * cx=0x05c47800, unsigned char * pc=0x05f5ce20, long * result=0x0012e360)  Line 3992 + 0xf bytes	C
js3250.dll!js_Invoke(JSContext * cx=0x05c47800, unsigned int argc=2, unsigned int flags=6)  Line 1325 + 0x13 bytes	C
js3250.dll!fun_apply(JSContext * cx=0x05c47800, JSObject * obj=0x03f09880, unsigned int argc=2, long * argv=0x0771b588, long * rval=0x0012e414)  Line 1713 + 0xf bytes	C
js3250.dll!js_Invoke(JSContext * cx=0x05c47800, unsigned int argc=2, unsigned int flags=0)  Line 1306 + 0x20 bytes	C
js3250.dll!js_Interpret(JSContext * cx=0x05c47800, unsigned char * pc=0x07392681, long * result=0x0012eaac)  Line 3992 + 0xf bytes	C
js3250.dll!js_Invoke(JSContext * cx=0x05c47800, unsigned int argc=1, unsigned int flags=2)  Line 1325 + 0x13 bytes	C
js3250.dll!js_InternalInvoke(JSContext * cx=0x05c47800, JSObject * obj=0x08c75020, long fval=136860832, unsigned int flags=0, unsigned int argc=1, long * argv=0x0771b1f8, long * rval=0x0012ec24)  Line 1400 + 0x14 bytes	C
js3250.dll!JS_CallFunctionValue(JSContext * cx=0x05c47800, JSObject * obj=0x08c75020, long fval=136860832, unsigned int argc=1, long * argv=0x0771b1f8, long * rval=0x0012ec24)  Line 4855 + 0x1f bytes	C
xul.dll!nsJSContext::CallEventHandler(nsISupports * aTarget=0x076551f8, void * aScope=0x058e8360, void * aHandler=0x082854a0, nsIArray * aargv=0x08dc0798, nsIVariant * * arv=0x0012ed90)  Line 1825 + 0x24 bytes	C++
xul.dll!nsJSEventListener::HandleEvent(nsIDOMEvent * aEvent=0x061de0d8)  Line 215 + 0x67 bytes	C++
xul.dll!nsEventListenerManager::HandleEventSubType(nsListenerStruct * aListenerStruct=0x0779d430, nsIDOMEventListener * aListener=0x062527a8, nsIDOMEvent * aDOMEvent=0x061de0d8, nsISupports * aCurrentTarget=0x076551f8, unsigned int aPhaseFlags=2)  Line 1092 + 0x12 bytes	C++
xul.dll!nsEventListenerManager::HandleEvent(nsPresContext * aPresContext=0x05be5788, nsEvent * aEvent=0x0012f408, nsIDOMEvent * * aDOMEvent=0x0012f060, nsISupports * aCurrentTarget=0x076551f8, unsigned int aFlags=2, nsEventStatus * aEventStatus=0x0012f064)  Line 1212	C++
xul.dll!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=2)  Line 202	C++
xul.dll!nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6, nsDispatchingCallback * aCallback=0x0012f114)  Line 283	C++
xul.dll!nsEventDispatcher::Dispatch(nsISupports * aTarget=0x08844628, nsPresContext * aPresContext=0x05be5788, nsEvent * aEvent=0x0012f408, nsIDOMEvent * aDOMEvent=0x00000000, nsEventStatus * aEventStatus=0x0012f208, nsDispatchingCallback * aCallback=0x0012f114)  Line 473 + 0x12 bytes	C++
xul.dll!PresShell::HandleEventInternal(nsEvent * aEvent=0x0012f408, nsIView * aView=0x073f5a30, nsEventStatus * aStatus=0x0012f208)  Line 5638 + 0x29 bytes	C++
xul.dll!PresShell::HandleEvent(nsIView * aView=0x073f5a30, nsGUIEvent * aEvent=0x0012f408, nsEventStatus * aEventStatus=0x0012f208)  Line 5439 + 0x1a bytes	C++
xul.dll!nsViewManager::HandleEvent(nsView * aView=0x073f5a30, nsPoint aPoint={...}, nsGUIEvent * aEvent=0x0012f408, int aCaptured=0)  Line 1285	C++
xul.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x0012f408, nsEventStatus * aStatus=0x0012f348)  Line 1238 + 0x22 bytes	C++
xul.dll!HandleEvent(nsGUIEvent * aEvent=0x0012f408)  Line 171	C++
xul.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012f408, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1112 + 0xc bytes	C++
xul.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0012f408)  Line 1133	C++
xul.dll!nsWindow::DispatchKeyEvent(unsigned int aEventType=131, unsigned short aCharCode=0, unsigned int aVirtualCharCode=13, long aKeyData=1835009, unsigned int aFlags=0)  Line 3475 + 0x11 bytes	C++
xul.dll!nsWindow::OnKeyDown(unsigned int aVirtualKeyCode=13, unsigned int aScanCode=28, long aKeyData=1835009)  Line 3696	C++
xul.dll!nsWindow::ProcessMessage(unsigned int msg=256, unsigned int wParam=13, long lParam=1835009, long * aRetValue=0x0012f924)  Line 4625 + 0x1d bytes	C++
xul.dll!nsWindow::WindowProc(HWND__ * hWnd=0x00090768, unsigned int msg=256, unsigned int wParam=13, long lParam=1835009)  Line 1325 + 0x1d bytes	C++
user32.dll!75b01a10()
[Frames below may be incorrect and/or missing, no symbols loaded for user32.dll]	
user32.dll!75b01ae8()
user32.dll!75b01a91()
user32.dll!75b02a47()
user32.dll!75afba9e()
user32.dll!75b02a98()
xul.dll!nsAppShell::ProcessNextNativeEvent(int mayWait=1)  Line 149	C++
xul.dll!nsBaseAppShell::DoProcessNextNativeEvent(int mayWait=1)  Line 137 + 0x11 bytes	C++
xul.dll!nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal * thr=0x01344018, int mayWait=1, unsigned int recursionDepth=0)  Line 247 + 0xf bytes	C++
xul.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012fb24)  Line 472	C++
xul.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x01344018, int mayWait=1)  Line 227 + 0x16 bytes	C++
xul.dll!nsBaseAppShell::Run()  Line 154 + 0xb bytes	C++
xul.dll!nsAppStartup::Run()  Line 171 + 0x1c bytes	C++
xul.dll!XRE_main(int argc=1, char * * argv=0x014cc3f0, const nsXREAppData * aAppData=0x004037e0)  Line 2868 + 0x25 bytes	C++
firefox.exe!main(int argc=1, char * * argv=0x014cc3f0)  Line 69 + 0x13 bytes	C++
firefox.exe!__tmainCRTStartup()  Line 597 + 0x19 bytes	C
firefox.exe!mainCRTStartup()  Line 414	C
kernel32.dll!761d3833()
ntdll.dll!771fa9bd()

Comment 11

[[e-mail]]

This is the crash that the patch is supposed to fix...

Comment 12

[Robert Strong (they/them - no direct email)]

I did some additional testing tonight and it appears that every so often it doesn't crash for me with the patch though as to why I don't know... I'll try to figure out why as time permits.

Comment 13

[Robert Strong (they/them - no direct email)]

I applied the patch to the EM in a branch build from 2007062707 and got the following Talkback
TB33803231G

Evaluating the following a couple of times in a trunk debug build will usually (guesstimate about 3 out of 4 times) reproduce the crash on my system though sometimes I have to restart for it to crash.
Components.classes['@mozilla.org/extensions/manager;1'].getService(Components.interfaces.nsIExtensionManager).datasource; Components.classes['@mozilla.org/rdf/rdf-service;1'].getService(Components.interfaces.nsIRDFService).GetDataSource('rdf:extensions');

If

Components.classes['@mozilla.org/rdf/rdf-service;1'].getService(Components.interfaces.nsIRDFService).GetDataSource('rdf:extensions');

is called before

Components.classes['@mozilla.org/extensions/manager;1'].getService(Components.interfaces.nsIExtensionManager).datasource;

is called I no longer crash

Perhaps GC?

Comment 14

[Robert Strong (they/them - no direct email)]

still investigating... registering the datasource after the call to loadExtensions appears to fix the crash for me.

Comment 15

[[e-mail]]

Bah, I hadn't tried reversing the order of those calls.
Thanks for spotting that. I'll try to see if I can find anything too.

Comment 16

[Robert Strong (they/them - no direct email)]

btw: I changed the order with your patch applied so I am still using nsISupportsInterfacePointer. I did a quick test without using nsISupportsInterfacePointer and using the changed order and it still crashed so the nsISupportsInterfacePointer is most likely necessary.

Comment 17

[[e-mail]]

Attachment: Fixed patch

For some reason, I can't seem to get a non-GCable pointer via nsISupportsInterfacePointer... this way seems a bit clunky, but it works...

Comment 18

[Robert Strong (they/them - no direct email)]

Comment on attachment 271388 [details] [diff] [review]
Fixed patch

r=me and thanks for taking care of this!

Comment 19

[[e-mail]]

Fix checked in, thanks!

Comment 20

[[e-mail]]

Attachment: Oops

I thought I tested getting the data source each way, but obviously not.
I think the simplest thing to do is just not to cache the datasource.
GetDataSource("rdf:extensions") still works but via the factory instead.

Comment 21

[Robert Strong (they/them - no direct email)]

I am certain I tried getting the datasource via the EM's datasource and via rdf:extensions. How are you seeing it failing?

Comment 22

[Robert Strong (they/them - no direct email)]

Attachment: JS for EM datasource test

I've been using the attached JS to test the datasource. On first run I can call this multiple times successfully on Firefox (displays an alert with Firefox (default) e.g. the name of the default theme) prior to opening the add-ons manager as well as after opening the add-ons manager. I've also verified once that the datasource successfully loads on app upgrade. Is there a scenario where loading the datasource fails that I am missing?

Comment 23

[[e-mail]]

I don't know about branch but on trunk when I try to get rdf:extensions I get
Error: uncaught exception: [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIRDFService.GetDataSource]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: file:///C:/mozilla/dist/bin/components/nsExtensionManager.js :: anonymous :: line 5820"  data: no]

Comment 24

[Robert Strong (they/them - no direct email)]

Strange, I was testing the patch on trunk as well as with a nightly trunk tbox build. I'll take a look today.

Comment 25

[(not reading, please use [e-mail] instead)]

fwiw, attachment #271457 [details] in venkman on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007070904 Minefield/3.0a7pre works as expected:

I get an alert with "Firefox (default)"

I did that before open the EM.

Comment 26

[[e-mail]]

It turns out that this is a debug-only check -
everything "works" OK in release builds.

Comment 27

[[e-mail]]

Attachment: Fix debug-only warning (Reentrant GS) on first RDF.GetDataSource("rdf:extensions")

Comment 28

[[e-mail]]

Debug fix checked in.

Comment 29

[Dave Townsend [:mossop]]

We don't have the extensions datasource anymore so no point in testing this