Closed
Bug 268089
Opened 20 years ago
Closed 13 years ago
After import setting and folders from mozilla 1.7, line 451: 25228 Segmentation fault
Categories
(Thunderbird :: Migration, defect)
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: slava, Assigned: asac)
Details
(Keywords: crash, Whiteboard: [patchlove])
Attachments
(2 obsolete files)
User-Agent: Mozilla/5.0 (X11; U; Linux i686; rv:1.7.3) Gecko/20041001 Firefox/0.10.1 Build Identifier: version 0.9 (20041103) After import mail folders and preferences from Mozilla 1.7, the Thunderbird 0.9 (and 0.8 too) crush: /usr/local/libexec/thunderbird_09/run-mozilla.sh: line 451: 25228 Segmentation fault (core dumped) "$prog" ${1+"$@"} I was try to delete mail folders and restart Thunderbird. The results is the same. I think, that problem in the prefs.js, generated after import mail boxes and setting. I can send this prefs.js and also core to you, but I don't can to put it here directly, because it is all my e-mail addresses in it. I have very many folders and subfolders and many accounts, and possibly some problem appear with such not simple configuration. Reproducible: Always Steps to Reproduce: With prefs.js which generating by Thunderbird after import setting, for reproducing bug it's need to do this: 1) Replace prefs.js at prefs.js from me. 2) Delete all from Mail directory 3) Start Thunderbird. 4) have segmentation fault and core dumped
Comment 1•19 years ago
|
||
I am having the same problem as well on Fedora Core 2 when converting from Mozilla Suite 1.7.3 to Thunderbird 1.0.
Comment 2•19 years ago
|
||
I have the same probleme too... Please contact me if I can help you by executing some tests. It's working fine (1.0.2) if you are not importing Mozilla data, but going to a core dump on starting after mozilla data importation. Can someone remove the "UNCONFIRMED" flag, and go to Blocker instead of Critical, since we are no more able to use Thunderbir unless this issues is corrected? And don't forget to vote for this bug... ;) Thanks, regards, Jean-Marc Spaggiari PS: Same as bug 285305 https://bugzilla.mozilla.org/show_bug.cgi?id=285305
Comment 3•19 years ago
|
||
Look like bug 268089 too... JMS
Comment 4•19 years ago
|
||
I can confirm this bug. Thunderbird segfaults during import of a Moziia Suite Profile. Since importing of the profile fails, and thunderbird starts normally with the create profile dialog afterwards, this is not a dupe of bug 285305. see also: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=285728
Comment 5•19 years ago
|
||
Deleting this line from my prefs.js solved my problem: (where mail.server.server1 is an imap server): user_pref("mail.server.server1.newsrc.file", 0); an otherwise empty prefs.js containing only this line reproduces this bug. Matthias.
Assignee | ||
Comment 6•19 years ago
|
||
a fix/workaround, that works for the case where the crash is triggered by a user_pref(mail.server.serverX.newsrc.file,0) in your mozilla profile. Please review the patch and let me know if you have better suggestions.
Attachment #186030 -
Flags: review?(benjamin)
Assignee | ||
Comment 7•19 years ago
|
||
Hmmm, just saw that the title is about a crash *after* importing mail and settings. The patch I have attached fixes a bug that occurrs *during* import. I can open a new bug if you want. Maybe someone has a better bug report that is already open for the bug that can be triggered like described in https://bugzilla.mozilla.org/show_bug.cgi?id=268089#c5 ? Otherwise, since 285305 already covers the *after* import crash, maybe we can rename this bug to 'crash during import'?
Comment on attachment 186030 [details] [diff] [review] patch that works around this problem 1. please post cvs diff -up patches instead of just diffs, they make things easier on everyone. >+ nsCOMPtr<nsILocalFile> srcNewsRCFile = do_CreateInstance(NS_LOCAL_FILE_CONTRACTID); 2. this do_CreateInstance can fail (i know you're just moving the line and that the code was bad to begin with, but it'd be nice if it was fixed), someone needs to null check srcNewsRCFile before we crash on the next line: >+ srcNewsRCFile->SetPersistentDescriptor(nsDependentCString(pref->stringValue)); >+ srcNewsRCFile->Exists(&exists); >+ if (exists) >+ { >+ nsCRT::free(pref->stringValue); 3. nsCRT and ToNewCString aren't compatible, this means someone's mixing allocators which is a no no (again, i realize this isn't your code). >+ pref->stringValue = ToNewCString(descriptorString); 4. trailing whitespace, whether you're moving code or adding it, be sure there isn't trailing whitespace: >+ }
Assignee | ||
Comment 9•19 years ago
|
||
> >+ nsCRT::free(pref->stringValue);
> 3. nsCRT and ToNewCString aren't compatible, this means someone's mixing
> allocators which is a no no (again, i realize this isn't your code).
> >+ pref->stringValue = ToNewCString(descriptorString);
OK, but which pattern is better? I found that this pattern is used throughout
the whole source file, so maybe we should fix it at all places.
Assignee | ||
Comment 10•19 years ago
|
||
this patch includes timeless' improvements nsCRT::free(pref->stringValue); pref->stringValue = ToNewCString(descriptorString); is still the same .... I looked through the code and found that this pattern is used more than one time. How can we improve this?
Assignee | ||
Updated•19 years ago
|
Attachment #186030 -
Attachment is obsolete: true
Attachment #186085 -
Flags: review?(benjamin)
Updated•19 years ago
|
Attachment #186030 -
Flags: review?(benjamin)
Comment 11•19 years ago
|
||
Comment on attachment 186085 [details] [diff] [review] improved patch This code was bad to begin with, but you need to make it better. Most importantly, you need to check the result from ->Exists and ->SetPersistentDescriptor: using the exists test without checking rv is a recipe for odd disasters.
Attachment #186085 -
Flags: review?(benjamin) → review-
Comment 12•19 years ago
|
||
the data that goes into this is XPCOM managed, so using nsCRT was wrong, the proper release is nsMemory/NS_Free. http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/modules/libpref/src/nsPrefBranch.cpp&rev=1.63&mark=548,579,587,592#547
Comment 13•19 years ago
|
||
Could someone please declare this Bug confirmed (since it is reproduceable in current firefox) and test if the latest patch fixes the bug? Matthias.
Comment 14•19 years ago
|
||
This is an automated message, with ID "auto-resolve01". This bug has had no comments for a long time. Statistically, we have found that bug reports that have not been confirmed by a second user after three months are highly unlikely to be the source of a fix to the code. While your input is very important to us, our resources are limited and so we are asking for your help in focussing our efforts. If you can still reproduce this problem in the latest version of the product (see below for how to obtain a copy) or, for feature requests, if it's not present in the latest version and you still believe we should implement it, please visit the URL of this bug (given at the top of this mail) and add a comment to that effect, giving more reproduction information if you have it. If it is not a problem any longer, you need take no action. If this bug is not changed in any way in the next two weeks, it will be automatically resolved. Thank you for your help in this matter. The latest beta releases can be obtained from: Firefox: http://www.mozilla.org/projects/firefox/ Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html Seamonkey: http://www.mozilla.org/projects/seamonkey/
Assignee | ||
Comment 15•19 years ago
|
||
This is 100% reproducible ... with doing https://bugzilla.mozilla.org/show_bug.cgi?id=268089#c5 ... so please someone confirm this, so it does not get lost. At some point I will submit an improved patch.
Comment 16•19 years ago
|
||
Confirming. I've also upgraded Alex's permissions so he can confirm bugs himself, and file new bugs directly as NEW. Gerv
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•17 years ago
|
QA Contact: migration
Comment 17•17 years ago
|
||
asac, is this still an issue with current trunk? If so, could you follow-up with a new patch to address the review comments?
Assignee: mscott → asac
Comment 18•15 years ago
|
||
Mark, patch is still relevant to versions we still migrate, correct? (In reply to comment #17) > asac, is this still an issue with current trunk? I think asac is gone.
Assignee | ||
Comment 19•15 years ago
|
||
unfortunately i lost my test profile i used to reproduce this. from patch seems it was reproducable by setting pref srcNewsRCFile or newsrc.file=0 ... someone wants to check that on comm-central builds?
Updated•15 years ago
|
Whiteboard: [patchlove]
Comment 21•15 years ago
|
||
Comment on attachment 186085 [details] [diff] [review] improved patch Setting obsolete due to r-, an updated patch is welcomed though.
Attachment #186085 -
Attachment is obsolete: true
Comment 22•13 years ago
|
||
I could not reproduce per comment 5 steps using current trunk
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•