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 on attachment 267132 [details] [diff] [review] Proposed patch Trying a different reviewer.

Comment on attachment 267132 [details] [diff] [review] Proposed patch Neil, I should be able to get to this tonight. Thanks!

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.

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

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

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

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

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

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

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.

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?

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

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.

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.

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 on attachment 271388 [details] [diff] [review] Fixed patch r=me and thanks for taking care of this!

Fix checked in, thanks!

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.

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

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?

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]

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

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.

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

Debug fix checked in.

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