Closed
Bug 752215
Opened 12 years ago
Closed 12 years ago
Debug MozMill tests fail with assertion 'i < Length() (invalid array index)'
Categories
(MailNews Core :: Database, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 15.0
People
(Reporter: standard8, Assigned: standard8)
Details
Attachments
(1 file, 1 obsolete file)
3.44 KB,
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
We're seeing an assertion on the debug MozMill tests: Assertion failure: i < Length() (invalid array index), at ../../dist/include/nsTArray.h:558 The top of the stack for this is: 0 libpthread-2.11.so + 0xee6b rbx = 0x00000000 r12 = 0xd9947a80 r13 = 0x00000000 r14 = 0xd83c2850 r15 = 0x02ba4370 rip = 0xd360ee6b rsp = 0xd673dc78 rbp = 0xd673dc90 Found by: given as instruction pointer in context 1 libxul.so!nsTArray<unsigned int, nsTArrayDefaultAllocator>::ElementAt [try-comm-central::1b45629067a3 : 558 + 0x37] rip = 0x007d7b75 rsp = 0xd673dc80 Found by: stack scanning 2 libxul.so!nsTArray<unsigned int, nsTArrayDefaultAllocator>::operator[] [try-comm-central::1b45629067a3 : 591 + 0x10] rip = 0x007d65b4 rsp = 0xd673dca0 Found by: stack scanning 3 libxul.so!nsMsgDatabase::MarkAllRead [nsMsgDatabase.cpp : 2662 + 0x10] rip = 0x01c1afe9 rsp = 0xd673dcc0 Found by: stack scanning 4 libxul.so!nsMsgDatabase::MarkHdrNotNew [nsMsgDatabase.cpp : 2628 + 0x6] rip = 0x01c1ad38 rsp = 0xd673dd30 Found by: stack scanning 5 0x7fffd673dd7f rbx = 0xd673e000 rip = 0xd673dd80 rsp = 0xd673dd38 rbp = 0x01c1ad38 Found by: call frame info 6 libxul.so!nsMsgDBFolder::MarkAllMessagesRead [nsMsgDBFolder.cpp : 1467 + 0x2c] rip = 0x019d98df rsp = 0xd673dd40 Found by: stack scanning The issue is in MarkAllRead - we're not checking the length of the nsTArray element before trying to copy the memory from it. I suspect we just generally get away with it in release mode, but we should fix it up anyway. I believe that before the recent bustage, this was the only thing to get our debug MozMill builders green. I'm attaching a patch that I think will fix it, I'm not altogether sure it is doing the right thing with the return values.
Attachment #621294 -
Flags: review?(dbienvenu)
Comment 1•12 years ago
|
||
Comment on attachment 621294 [details] [diff] [review] Proposed fix This will cause us to generate a bogus url if you try to mark all read in an imap folder with no unread messages. Although this might be harmless, I tend to think we should either make this an error, or change the callers to check for a 0 *aNumKeys, e.g., nsImapMailFolder::MarkAllMessagesRead and nsImapMailFolder::MarkThreadRead.
Attachment #621294 -
Flags: review?(dbienvenu) → review-
Comment 2•12 years ago
|
||
I'd be r+ happy with a patch that added a couple lines to StoreImapFlags to make it just return an error if numKeys is 0.
Assignee | ||
Comment 3•12 years ago
|
||
I took another look at this, the problem I have with doing a change in StoreImapFlags is that it looks like we'd still get an undo change item created which is probably not what we want. So I've just added a check to ensure that we've actually changed something. In nsMsgDBFolder::MarkAllMessagesRead I also moved an ensure success check so that we'd re-enable notifications even if MarkAllRead failed for some reason.
Attachment #621294 -
Attachment is obsolete: true
Attachment #622359 -
Flags: review?(dbienvenu)
Comment 4•12 years ago
|
||
Comment on attachment 622359 [details] [diff] [review] Proposed fix v2 looks reasonable.
Attachment #622359 -
Flags: review?(dbienvenu) → review+
Assignee | ||
Comment 5•12 years ago
|
||
Checked in: https://hg.mozilla.org/comm-central/rev/f73fb84d8279
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0
You need to log in
before you can comment on or make changes to this bug.
Description
•