Closed
Bug 76914
Opened 24 years ago
Closed 24 years ago
Sorting in mail by Sender or Subject causes browser to core dump.
Categories
(SeaMonkey :: MailNews: Message Display, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.2
People
(Reporter: BarrettLndstrm, Assigned: sspitzer)
References
Details
Attachments
(8 files)
7.17 KB,
text/plain
|
Details | |
7.60 KB,
text/plain
|
Details | |
813 bytes,
patch
|
Details | Diff | Splinter Review | |
1.67 KB,
patch
|
Details | Diff | Splinter Review | |
1.69 KB,
patch
|
Details | Diff | Splinter Review | |
908 bytes,
patch
|
Details | Diff | Splinter Review | |
1.26 KB,
patch
|
Details | Diff | Splinter Review | |
1.29 KB,
patch
|
Details | Diff | Splinter Review |
This has been reproduced in both mozilla and netscape daily trunk builds
#2001041921.
If you click on either the Sender or Subject tab to sort the messages in mail,
the browser will coredump.
STEPS TO REPRODUCE:
1. Start N6 (or mozilla) mail.
2. After you log in, click on either the sender tab or the subject tab.
EXPECTED RESULTS: The mail messages should sort by the tab that was clicked.
ACTUAL RESULTS: The browser core dumps.
I'm setting the componant to Mail window front end, because I have no idea what
to set it to, and Bugzilla won't let me leave it not set.
Reporter | ||
Comment 1•24 years ago
|
||
Updated qa contact and adding blocker....
Blocks: 18687
QA Contact: esther → barrettl
Reporter | ||
Comment 3•24 years ago
|
||
hmmmmm. Seems similar, but not quite the same. On HPUX, you only have to click
the subject or sender tab once, and the browser will core dump every time
without fail. Also, if your mail has been set to sort by either subject or
sender previously (using an older N6 to change the sort and then exiting), as
soon as it trys to get the messages from the server, the browser will core dump.
Since this doesn't happen on linux, I can't see it as being a non-platform
specific bug. It would be strange to have a bug that affected only Mac and
Hpux, but I suppose stranger things have happend. I will add these comments to
bug 76919. shannond, any thoughts on this?
Comment 4•24 years ago
|
||
Comment 5•24 years ago
|
||
I suppose that its possible that this could be related to bug 76919, but lets
keep them separate for now. I'll keep an eye on the other bug
Comment 6•24 years ago
|
||
This problem was introduced somewhere between 2/26/2001 and 3/30/2001 ... still
narrowing down ...
Comment 7•24 years ago
|
||
narrowed down to between 3/15 and 3/22
Comment 8•24 years ago
|
||
Looks like this problem started after the mailnews performance branch landing
Comment 9•24 years ago
|
||
Comment 10•24 years ago
|
||
to answer Shannon's question, no we aren't seeing this crash on other platforms.
seems to be HPUX only. You posted a stack trace, but what's going on in the
stack trace? i.e. where in the sort method are you crashing? Is something null
or is a piece of memory corrupted?
Comment 11•24 years ago
|
||
The crash occurs in this same spot in the code while in the process of the sort.
Depending on the contents of my inbox the crash could occur at any random
message - sometimes it crashes on the first message (numSoFar = 0)... sometimes
it crashes on the fifth ... etc. But I havent' been able to find a pattern for
why it crashes where it does and on which message this happens. I haven't found
anything to be null and am still trying to determine if something is corrupted.
Status: NEW → ASSIGNED
Comment 12•24 years ago
|
||
I will take a look at this bug.
Comment 13•24 years ago
|
||
Comment 14•24 years ago
|
||
The problem is caused by memory alignment. On HPUX, int32 need to be accessed in 4-byte
boundary. Otherwise it will crash the system. We need to bear this in mind. It is a
virtue of our HPUX.
shannon, please take care of the rest stuff (r=, sr=, checkin).
Assignee | ||
Comment 15•24 years ago
|
||
I'll do the review / super-review.
Updated•24 years ago
|
Target Milestone: --- → mozilla0.9.1
Assignee | ||
Comment 16•24 years ago
|
||
I have problems with this fix.
1) first, you've only fixed one of the callers GetFieldTypeAndLenForSort().
there are multiple callers. instead of adjusting the count afterwards, let's
make the default values be the right size.
(kMaxSubjectKey, kMaxLocationKey, kMaxAuthorKey, kMaxRecipientKey)
if you do change the size, include a comment about why the size has to be a
multiple of whatever. (so when it gets changed next time, we won't break HPUX)
2)
+ //memory alignment before we adjust bTemp. Otherwise it will crash on some
system
+ bytesToCopy += (4-bytesToCopy%4);
shouldn't you be updating bytesToCopy before doing the memcpy?
before we check any patch it, we'll need to test it on linux, mac and win32.
these remind me of a problem I had with sizeof(EntryInfo) not returning what I
thought it would. see
http://mail-index.netbsd.org/current-users/1995/07/12/0018.html
for an example of what I'm talking about.
to summarize, I'm not happy with all the crazy pointer arithmetic that goes on
nsMsgDBView.cpp. it's just an implementation of an arena. I'd like to clean
it up to use the arenas we've already got in mozilla, but until then, I'll
gladly take a fix to make this work on HPUX.
please come up with a cleaner fix. if you are not able to test it on win32,
linux and mac, let me know what platforms you can test on and I'll cover the
rest.
Comment 17•24 years ago
|
||
Yes, modifying the max len constant is better. (I did not know they
are from constants.) Multiple callers? Please check it to see what
Seth mean and fix it. Modify size before memory copy?
I do not think so! You don't want to access memory outside its desired range.
(Probably I should not reuse variable "bytesToCopy".)
Put something like:
PRInt32 bytesToPad = 4 - bytesToCopy & 0x00000003;
if (bytesToPad == 4) bytesToByte = 0;
Then increase the amounf of (bytesToCopy+bytesToPad).
Assignee | ||
Comment 18•24 years ago
|
||
I'm not sure I understand all your comments, but I'll just wait for the next patch.
one more comment: in the next version of the patch, instead of doing "4" do
sizeof(PRInt32).
Comment 19•24 years ago
|
||
Comment 20•24 years ago
|
||
I've attached a patch created from Seth's and Shanjian's comments. This works
on HP-UX and doesn't bust sorting on Linux.
Comment 21•24 years ago
|
||
Comment 22•24 years ago
|
||
I do not understand why you want to add additional 4 to those constant.
That's not necessary. Everything else looks fine to me.
Comment 23•24 years ago
|
||
The first part of my patch does the same thing as the first part of Shanjian's
patch only to the constants to which maxLen is assigned as opposed to performing
the alignment on maxLen after it is returned. However, I see that this is
unnecessary because those values (kMaxSubjectKey, kMaxLocationKey,
kMaxAuthorKey, kMaxRecipientKey) do not require alignment (are already divisible
by 4). Therefore, only the second part of my patch is necessary. Is this
correct?
Comment 24•24 years ago
|
||
Reassigning to Shanjian based on his experience with this problem on HP-UX
Assignee: shannond → shanjian
Status: ASSIGNED → NEW
Updated•24 years ago
|
Assignee: shanjian → shannond
Comment 25•24 years ago
|
||
reassign to shannon.
Comment 26•24 years ago
|
||
shannon, please update your patch. Ater that, r=shanjian.
Assignee | ||
Comment 27•24 years ago
|
||
is the final patch simply going to be changing the constants and adding a
comment explaining why those values are necessary for HPUX?
Comment 28•24 years ago
|
||
Sorry, I am not the owner of this module, I shouldn't give r=. I withdraw.
Seth, could you give both r/sr? We really want to fix this ASAP.
Comment 29•24 years ago
|
||
Comment 30•24 years ago
|
||
Adjusted patch as requested by Shanjian ... Is this still correct Shanjian?
Seth, is this okay with you? This fixes the problem on HPUX and doesn't seem to
have any ill affects on Linux
Status: NEW → ASSIGNED
Comment 31•24 years ago
|
||
Comment 32•24 years ago
|
||
We need to put some comment to place where those constants are defined.
That's to make sure people do not break it when they change the value.
Assignee | ||
Comment 33•24 years ago
|
||
the patch keeps changing. I thought the fix was to change the constants and put
comments explaining why they had to be a certain multiple of X so that HPUX
wouldn't crash.
I'm the proper owner of this, I'll drive it in. but the patch needs to settle
down first.
Assignee: shannond → sspitzer
Status: ASSIGNED → NEW
Comment 34•24 years ago
|
||
Those constants are already in 4-byte boundary. But we do need to put some comment
there to make sure future change will not break this. That's why I updated shannon's
latest patch. Other than that, they are the same.
Comment 36•24 years ago
|
||
Please try to resolve this in 0.9.1. This has great impact on HPUX.
The last patch will be the final, unless you have other suggestions.
Assignee | ||
Comment 37•24 years ago
|
||
please test the final patch on mac, win and linux.
it's on my list of fixes to review today.
Comment 38•24 years ago
|
||
Assignee | ||
Comment 39•24 years ago
|
||
r/sr=sspitzer
assuming testing goes well, check it in.
Comment 40•24 years ago
|
||
checked in
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 41•24 years ago
|
||
Verified fixed on build # 2001052021 on hpux 11i.
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•