Closed Bug 457052 Opened 14 years ago Closed 14 years ago

try to repair cause of assertion - Borked message header


(MailNews Core :: Database, defect)

Not set


(Not tracked)

Thunderbird 3.0b1


(Reporter: Bienvenu, Assigned: Bienvenu)


(Blocks 1 open bug)



(2 files, 1 obsolete file)

Attached patch proposed fix (obsolete) — Splinter Review
Dan, especially, has been seeing a lot of these. The problem is that a message's thread parent isn't valid, i.e., it's not in the thread. The upcoming patch attempts to repair this situation, and changes the assertion to a warning.

The assertion appears harmless, but I think in some situations it could cause us not to display sub-messages in threaded view (not sure, though).

This patch repairs the problem for me. I still have to figure out why this is happening, however.
Attachment #340385 - Flags: superreview?(dmose)
Attachment #340385 - Flags: review?(bugzilla)
I believe we decided we wanted this for 3.0b1; marking as such.
Flags: blocking-thunderbird3.0b1+
Target Milestone: --- → Thunderbird 3.0b1
Running with this patch, I'm seeing a small number of the "Thread is a parent of itself" assertions (I was seeing them before, too, I think).  Should that really be an assertion, or should it be similar to the problem already solved in this patch and warn and rebuild the index instead...
Comment on attachment 340385 [details] [diff] [review]
proposed fix 

This is making the method big enough that it's becoming difficult to digest.  How about splitting it apart?
Attachment #340385 - Flags: superreview?(dmose) → superreview-
ok, this moves the code into a new method. I don't have a test case anymore, though, so I can't swear that this works. Dan, can you try this out and see if you hit the code, and it fixes the problem?
Attachment #340385 - Attachment is obsolete: true
Attachment #340749 - Flags: superreview?(dmose)
Attachment #340749 - Flags: review?(bugzilla)
Attachment #340385 - Flags: review?(bugzilla)
(In reply to comment #1)
> I believe we decided we wanted this for 3.0b1; marking as such.

Correcting flag to get it onto the right radar.
Flags: blocking-thunderbird3.0b1+ → blocking-thunderbird3+
Comment on attachment 340749 [details] [diff] [review]
proposed fix - checked in

This looks ok to me.
Attachment #340749 - Flags: review?(bugzilla) → review+
Whiteboard: [needs new patch] → [needs review dmose]
Comment on attachment 340749 [details] [diff] [review]
proposed fix - checked in

* It would be good to track stuff that should just warn and trigger database rebuilds once rkent's lossless-database-rebuild stuff is in, such as presumably the "Thread is parent of itself" assertion (and maybe the "no root hdr" assertion too?).  Please file a spinoff bug, if you would...

>+      nsMsgHdr* curMsgHdr = static_cast<nsMsgHdr*>(curHdr.get());      // closed system, cast ok

Please reformat so that this wraps at 80 columns.

Attachment #340749 - Flags: superreview?(dmose) → superreview+
fix checked in.

I'll file a spin-off bug, but I'm leaning towards repair as opposed to rebuilding.
Closed: 14 years ago
Resolution: --- → FIXED
This fixes a few other borked thread issues:

1. If a message is marked as the thread root key, but doesn't have a thread parent of -1, make GetRootHdr return the correct header (which will correct the thread root key)

2. If a message is in a thread object, but doesn't have its thread id set to the thread, correct it.

3. If a message is its own parent, reparent it in the thread.

4. In nsMsgThread::ReparentMsgsWithInvalidParent, make sure we don't set our parent to ourselves.

I'm not sure the last change is actually needed. The other changes fix the test cases I've got, I believe. I'll double check that.
Attachment #343621 - Flags: superreview?(neil)
Attachment #343621 - Flags: review?(bugzilla)
re-opening, and putting in beta 1.
Resolution: FIXED → ---
Target Milestone: Thunderbird 3.0a3 → Thunderbird 3.0b1
See bug 272527, which bienvenu thinks is fixed by the patch here.
There's an old attachment with an INBOX.msf, too.
Blocks: 272527
Attachment #343621 - Flags: superreview?(neil) → superreview+
Comment on attachment 343621 [details] [diff] [review]
fix some other related issues - checked in

>+            PRInt32 resultIndex;
>+            nsMsgKey rootKey;
>+            nsCOMPtr <nsIMsgDBHdr> root;
>+            GetRootHdr(&resultIndex, getter_AddRefs(root));
>+            root->GetMessageKey(&rootKey);
sr=me if you replace this and just use m_threadRootKey instead.
Attachment #343621 - Flags: review?(bugzilla) → review+
Whiteboard: [needs review dmose] → [needs comment addressing & landing]
fix checked in.
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Attachment #340749 - Attachment description: prposed fix → proposed fix - checked in
Attachment #343621 - Attachment description: fix some other related issues → fix some other related issues - checked in
OS: Windows XP → All
Hardware: PC → All
Whiteboard: [needs comment addressing & landing]
You need to log in before you can comment on or make changes to this bug.