2.14 KB, patch
|Details | Diff | Splinter Review|
Caveat: I haven't actually seen this in the debugger, but it's the only way I can interpret the data in the crash log, and there's certainly a bug here. In NS_NewLocalStore, we only fill aResult through the impl->QueryReference call, which is in turn only called if impl->Refresh is called. If impl->Refresh fails, then we're going to return the success code of the observer-service NS_WITH_SERVICE, which is likely true. I bet what happened is that whoever added the profile-switch observer stuff didn't realize that the code counted on falling through and returning the error from Refresh, in the case that it failed. The patch is left as an exercise to one of the readers, though I think switching to use of a comptr to hold impl's ref and bailing early if impl->Refresh fails is a decent plan.
(Adding crash keyword. Tip o' the hat to timeless for helping me analyze this.)
Jpatel/Namachi - Per PDT, can you look for NS_NewLocalStore in crash database, to see if we can find reports on it?
I don't see any crashes in Talkback data that have the NS_NewLocalStore stack signature. Has anyone been able to reproduce a crash related to this problem and get a stack trace? That might help find the right stack signature to search Talkback for.
This bug was found by analyzing the crash data pointed to by the URL associated with this bug report. What more do you need?
There have been a few crashes reported by Talkback with the nsServiceManagerImpl::GetService stack signature in recent builds: 2 crashes with build 2001052213, 2 with build 2001052115, 1 with build 2001051806 and a 8 crashes reported with build 2001051710. Here is the info for the latest crash: Incident ID 30830296 Trigger Time 2001-05-23 16:04:01 User Comments Launching on startup after loading a new theme. Build ID 2001052213 Platform ID Win32 Stack Trace nsServiceManagerImpl::GetService [d:\builds\seamonkey\mozilla\xpcom\components\nsServiceManager.cpp, line 368] nsServiceManager::GetService [d:\builds\seamonkey\mozilla\xpcom\components\nsServiceManager.cpp, line 560] nsXULDocument::Init [d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULDocument.cpp, line 3749] NS_NewXULDocument [d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULDocument.cpp, line 529] nsContentFactory::CreateInstance [d:\builds\seamonkey\mozilla\content\build\nsContentFactory.cpp, line 570] nsComponentManagerImpl::CreateInstance [d:\builds\seamonkey\mozilla\xpcom\components\nsComponentManager.cpp, line 1206] nsComponentManager::CreateInstance [d:\builds\seamonkey\mozilla\xpcom\components\nsRepository.cpp, line 82] nsLayoutDLF::CreateRDFDocument [d:\builds\seamonkey\mozilla\layout\build\nsLayoutDLF.cpp, line 442] nsLayoutDLF::CreateRDFDocument [d:\builds\seamonkey\mozilla\layout\build\nsLayoutDLF.cpp, line 468] nsLayoutDLF::CreateInstance [d:\builds\seamonkey\mozilla\layout\build\nsLayoutDLF.cpp, line 316] nsDocShell::NewContentViewerObj [d:\builds\seamonkey\mozilla\docshell\base\nsDocShell.cpp, line 3464] nsDocShell::CreateContentViewer [d:\builds\seamonkey\mozilla\docshell\base\nsDocShell.cpp, line 3365] nsDSURIContentListener::DoContent [d:\builds\seamonkey\mozilla\docshell\base\nsDSURIContentListener.cpp, line 120] nsDocumentOpenInfo::DispatchContent [d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp, line 353] nsDocumentOpenInfo::OnStartRequest [d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp, line 220] nsJARChannel::OnStartRequest [d:\builds\seamonkey\mozilla\netwerk\protocol\jar\src\nsJARChannel.cpp, line 566] nsOnStartRequestEvent::HandleEvent [d:\builds\seamonkey\mozilla\netwerk\base\src\nsRequestObserverProxy.cpp, line 109] PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591] PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 524] _md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 1072] nsAppShellService::Run [d:\builds\seamonkey\mozilla\xpfe\appshell\src\nsAppShellService.cpp, line 418] main1 [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1100] main [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1394] WinMain [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1412] WinMainCRTStartup() KERNEL32.DLL + 0x192a6 (0x77e992a6) And here are some comments: - crash on startup - Trying to start by clicking on Mozilla.exe - startup, sometime after the splash before the browser appears - started the browser
Created attachment 35967 [details] [diff] [review] integrate Refresh() and observer setup into Init()
If you go back to r1.10 of this file, I see that rjc added |impl->Refresh(PR_TRUE)| to NS_NewLocalStore(). I should've cleaned this up when I changed the signature of NS_NewLocalStore(), but didn't. This probably set the bad precedent which ccarlen just followed when he added the profile switching stuff. Anyway, the above patch moves both things to the Init() method, and hard-fails if we can't |Refresh(PR_TRUE)|.
r=alecf, very straight forward
hyatt sez, sr=hyatt! really!
a= email@example.com for checkin to 0.9.1
Fix checked in.
*** Bug 74765 has been marked as a duplicate of this bug. ***
verified fixed. this crash no longer shows up in Talkback data.
Sorry for the spam, adding M09 to summary since bug 74765 was marked a dup.