Last Comment Bug 371653 - crash in [@ nsMsgAccountManager::GetIncomingServer] when getPrefService has not been called
: crash in [@ nsMsgAccountManager::GetIncomingServer] when getPrefService has n...
: crash
Product: MailNews Core
Classification: Components
Component: Account Manager (show other bugs)
: Trunk
: x86 All
-- critical (vote)
: Thunderbird 13.0
Assigned To: :aceman
Depends on:
  Show dependency treegraph
Reported: 2007-02-25 14:17 PST by arno renevier
Modified: 2012-02-13 06:26 PST (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

call getPrefService in nsMsgAccountManager::GetIncomingServer (1.13 KB, patch)
2007-02-25 14:22 PST, arno renevier
no flags Details | Diff | Splinter Review
patch per comment 5 (13.89 KB, patch)
2012-01-23 13:31 PST, :aceman
standard8: review+
Details | Diff | Splinter Review
patch with nit (13.90 KB, patch)
2012-02-08 10:00 PST, :aceman
acelists: review+
Details | Diff | Splinter Review

Description User image arno renevier 2007-02-25 14:17:29 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20061205 Iceweasel/ (Debian-
Build Identifier: version 3 alpha 1 (20070225)

If nsMsgAccountManager::GetIncomingServer is called, but m_pref is null, 
rv = m_prefs->GetCharPref(serverPref.get(), getter_Copies(serverType));
makes thunderbird crash
This can happen if you have an application shipped as an extension for thunderbird (application which you call with thunderbird -chrome chrome://blabla/content), and first line of your script is for example:


I've attached a .xpi for testcase.

My suggestion is to add a call to nsMsgAccountManager::getPrefService in nsMsgAccountManager::GetIncomingServer.
I'll attach this solution as a patch.

Reproducible: Always

Steps to Reproduce:
1. install .xpi at
2. start thunderbird with thunderbird -chrome chrome://crash/content/crash.xul
Actual Results:  

Expected Results:  
no crash
Comment 1 User image arno renevier 2007-02-25 14:22:15 PST
Created attachment 256382 [details] [diff] [review]
call getPrefService in nsMsgAccountManager::GetIncomingServer
Comment 2 User image arno renevier 2007-02-26 01:16:12 PST
Another possibility would be to call getPrefService once in nsMsgAccountManager constructor instead of calling it in each method that needs it. That ensures we don't have the same problem on other methods. Are there any drawbacks ?
Comment 3 User image timeless 2007-02-26 03:50:34 PST
it can fail and constructors have no way of indicating failure.

so you can be right back where you started. if you don't mind crashing again next time, then it doesn't have any drawbacks...
Comment 4 User image Gary Kwong [:gkw] [:nth10sd] 2008-09-21 04:10:15 PDT
The .xpi can no longer be accessed.

Bienvenu, any thoughts on this? (not sure on the validity of this..)
Comment 5 User image Mark Banner (:standard8) 2008-09-21 05:08:53 PDT
The bug is essentially still valid. Given nsMsgAccountManager is a service and has an Init function, I would suggest that the pref service is initialised once in the Init function, and the getPrefService function is dropped.
Comment 6 User image Wayne Mery (:wsmwk, NI for questions) 2011-08-31 16:39:22 PDT
(no crashes in last 9 months)
Comment 7 User image :aceman 2012-01-23 13:29:04 PST
Should the same (comment 5) be done in nsMsgAccount.cpp too? It seems the only file using similar approach.
Comment 8 User image :aceman 2012-01-23 13:31:55 PST
Created attachment 590849 [details] [diff] [review]
patch per comment 5

Standard8, did you mean something like this?
Comment 9 User image Mark Banner (:standard8) 2012-02-07 15:51:55 PST
Comment on attachment 590849 [details] [diff] [review]
patch per comment 5

Review of attachment 590849 [details] [diff] [review]:

::: mailnews/base/src/nsMsgAccountManager.cpp
@@ +1273,5 @@
> +   * to add to the existing accounts list.
> +   */
> +  nsCOMPtr<nsIPrefBranch> defaultsPrefBranch;
> +  rv = prefservice->GetDefaultBranch(MAILNEWS_ROOT_PREF, getter_AddRefs(defaultsPrefBranch));
> +  NS_ENSURE_SUCCESS(rv,rv);

nit: as you're touching these lines, please insert a space after the comma (several places in this section)
Comment 10 User image :aceman 2012-02-08 10:00:38 PST
Created attachment 595442 [details] [diff] [review]
patch with nit

With review from standard8.
Comment 11 User image Mark Banner (:standard8) 2012-02-13 06:26:54 PST
Checked in:

Note You need to log in before you can comment on or make changes to this bug.