419471 - Crash [@nsPalmSyncSupport::GetPalmSyncInstall]

Status: RESOLVED FIXED

(MailNews Core Graveyard :: Palm Sync, defect)

Description

[Mark Banner (:standard8)]

Attachment: The fix

I picked this up off crash stats:

0  	nsPalmSyncSupport::GetPalmSyncInstall(nsILocalFile**)  	 d:\builds\tinderbox\seamonkeytrunk\winnt_5.2_depend\mozilla\mailnews\extensions\palmsync\src\nspalmsyncsupport.cpp:179
1 	nsPalmSyncSupport::LaunchPalmSyncInstallExe() 	d:\builds\tinderbox\seamonkeytrunk\winnt_5.2_depend\mozilla\mailnews\extensions\palmsync\src\nspalmsyncsupport.cpp:204
2 	nsPalmSyncSupport::Observe(nsISupports*, char const*, unsigned short const*) 	d:\builds\tinderbox\seamonkeytrunk\winnt_5.2_depend\mozilla\mailnews\extensions\palmsync\src\nspalmsyncsupport.cpp:122
3 	nsObserverList::NotifyObservers(nsISupports*, char const*, unsigned short const*) 	d:\builds\tinderbox\seamonkeytrunk\winnt_5.2_depend\mozilla\xpcom\ds\nsobserverlist.cpp:128
4 	nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) 	d:\builds\tinderbox\seamonkeytrunk\winnt_5.2_depend\mozilla\xpcom\ds\nsobserverservice.cpp:181
5 	nsXREDirProvider::DoStartup() 	d:\builds\tinderbox\seamonkeytrunk\winnt_5.2_depend\mozilla\toolkit\xre\nsxredirprovider.cpp:816
6 	XRE_main 	d:\builds\tinderbox\seamonkeytrunk\winnt_5.2_depend\mozilla\toolkit\xre\nsapprunner.cpp:3016
7 	NS_internal_main(int, char**) 	d:\builds\tinderbox\seamonkeytrunk\winnt_5.2_depend\mozilla\suite\app\nssuiteapp.cpp:103
8 	wmain 	d:\builds\tinderbox\seamonkeytrunk\winnt_5.2_depend\mozilla\toolkit\xre\nswindowswmain.cpp:87
9 	__tmainCRTStartup 	f:\sp\vctools\crt_bld\self_x86\crt\src\crtexe.c:594
10 	BaseProcessStart 	

Looks like a probable user playing around with profile (from description http://crash-stats.mozilla.com/report/index/94fb6970-db1d-11dc-96c0-001a4bd43ed6) but the cause is obvious (patch coming up) and we should fix it anyway.

The problem is getInstallLocation can return success, but a null install location - so we need to handle that case, patch attached which does that. Also changes getting the EM service result so that we return the actual failure rather than a generic one. I think if we get a null installLocation, NS_ERROR_FAILURE is the best we can do.

Comment 1

[David :Bienvenu]

NS_ENSURE_TRUE(rv, rv) seems wrong - you mean NS_ENSURE_SUCCESS, right?

Comment 2

[Mark Banner (:standard8)]

(In reply to comment #1)
> 
> NS_ENSURE_TRUE(rv, rv) seems wrong - you mean NS_ENSURE_SUCCESS, right?
> 

Yes I do. Sorry my build env isn't set up at the moment.

Comment 3

[David :Bienvenu]

Comment on attachment 305540 [details] [diff] [review]
The fix

ok, r/sr=me, with NS_ENSURE_TRUE changed to NS_ENSURE_SUCCESS(rv, rv);

Comment 4

[Mark Banner (:standard8)]

Patch checked in -> fixed.

Comment 5

[Wayne Mery (:wsmwk)]

(In reply to comment #0)
> Created an attachment (id=305540) [details]
> The fix
> 
> I picked this up off crash stats:
> 
> 0       nsPalmSyncSupport::GetPalmSyncInstall(nsILocalFile**)   
> d:\builds\tinderbox\seamonkeytrunk\winnt_5.2_depend\mozilla\mailnews\extensions\palmsync\src\nspalmsyncsupport.cpp:179

Mark, was it pure luck that you found this Seamonkey crash?


> Looks like a probable user playing around with profile (from description
> http://crash-stats.mozilla.com/report/index/94fb6970-db1d-11dc-96c0-001a4bd43ed6)

crash comments are "Trying to clean up a Profile, I deleted much from Profile. Upon restart, this crash. I'll delete everything, create an empty prefs.js, & see what that does."


> but the cause is obvious (patch coming up) and we should fix it anyway.
> 
> The problem is getInstallLocation can return success, but a null install
> location 

Do we know under what types of circumstances this might happen, that we get a null install location?  

Comment 6

[Wayne Mery (:wsmwk)]

The crash report is gone, but if this was on Vista, probability is high that this is related to bug 422494.
cc: Marc