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)

HP
HP-UX
defect
Not set
critical

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.2

People

(Reporter: BarrettLndstrm, Assigned: sspitzer)

References

Details

Attachments

(8 files)

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.
Updated qa contact and adding blocker....
Blocks: 18687
QA Contact: esther → barrettl
Please see if bug 76919 is a dupe of this.
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?
Attached file stack trace of crash (deleted) —
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
This problem was introduced somewhere between 2/26/2001 and 3/30/2001 ... still narrowing down ...
narrowed down to between 3/15 and 3/22
Looks like this problem started after the mailnews performance branch landing
Attached file updated stack trace (deleted) —
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?
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
I will take a look at this bug.
Attached patch proposed fix (deleted) — Splinter Review
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).
I'll do the review / super-review.
Target Milestone: --- → mozilla0.9.1
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.
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).
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).
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.
Attached patch better if-statement format (deleted) — Splinter Review
I do not understand why you want to add additional 4 to those constant. That's not necessary. Everything else looks fine to me.
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?
Reassigning to Shanjian based on his experience with this problem on HP-UX
Assignee: shannond → shanjian
Status: ASSIGNED → NEW
Assignee: shanjian → shannond
reassign to shannon.
shannon, please update your patch. Ater that, r=shanjian.
is the final patch simply going to be changing the constants and adding a comment explaining why those values are necessary for HPUX?
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.
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
Attached patch update shannon's patch (deleted) — Splinter Review
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.
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
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.
moving to 0.9.2
Target Milestone: mozilla0.9.1 → mozilla0.9.2
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.
please test the final patch on mac, win and linux. it's on my list of fixes to review today.
r/sr=sspitzer assuming testing goes well, check it in.
checked in
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Verified fixed on build # 2001052021 on hpux 11i.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: