Closed
Bug 371653
Opened 17 years ago
Closed 12 years ago
crash in [@ nsMsgAccountManager::GetIncomingServer] when getPrefService has not been called
Categories
(MailNews Core :: Account Manager, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 13.0
People
(Reporter: arno, Assigned: aceman)
References
()
Details
(Keywords: crash)
Crash Data
Attachments
(1 file, 2 obsolete files)
13.90 KB,
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1) Gecko/20061205 Iceweasel/2.0.0.1 (Debian-2.0.0.1+dfsg-2) 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: Components.classes["@mozilla.org/messenger/account-manager;1"]. createInstance(Components.interfaces.nsIMsgAccountManager). getIncomingServer('server1'); 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 http://ffsearchplugins.free.fr/bugzilla/nsimsgaccountmanager-crash.xpi 2. start thunderbird with thunderbird -chrome chrome://crash/content/crash.xul Actual Results: crash Expected Results: no crash
Reporter | ||
Comment 1•17 years ago
|
||
Updated•17 years ago
|
Reporter | ||
Comment 2•17 years ago
|
||
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 ?
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...
Updated•16 years ago
|
Assignee: mscott → nobody
Comment 4•16 years ago
|
||
The .xpi can no longer be accessed. Bienvenu, any thoughts on this? (not sure on the validity of this..)
Updated•16 years ago
|
Attachment #256382 -
Attachment is patch: true
Attachment #256382 -
Attachment mime type: application/octet-stream → text/plain
Comment 5•16 years ago
|
||
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.
Keywords: helpwanted
Summary: crash in nsMsgAccountManager::GetIncomingServer when getPrefService has not been called → crash in [@ nsMsgAccountManager::GetIncomingServer] when getPrefService has not been called
Updated•13 years ago
|
Crash Signature: [@ nsMsgAccountManager::GetIncomingServer]
Should the same (comment 5) be done in nsMsgAccount.cpp too? It seems the only file using similar approach.
Assignee: acelists → nobody
Status: NEW → ASSIGNED
Product: Thunderbird → MailNews Core
QA Contact: account-manager → account-manager
Whiteboard: [patchlove] [has draft patch]
Version: unspecified → Trunk
Standard8, did you mean something like this?
Assignee: nobody → acelists
Attachment #256382 -
Attachment is obsolete: true
Attachment #590849 -
Flags: review?(mbanner)
Comment 9•12 years ago
|
||
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)
Attachment #590849 -
Flags: review?(mbanner) → review+
Assignee | ||
Comment 10•12 years ago
|
||
With review from standard8.
Attachment #590849 -
Attachment is obsolete: true
Attachment #595442 -
Flags: review+
Keywords: helpwanted → checkin-needed
Comment 11•12 years ago
|
||
Checked in: http://hg.mozilla.org/comm-central/rev/e0ccaae60f48
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
You need to log in
before you can comment on or make changes to this bug.
Description
•