The default bug view has changed. See this FAQ.

assert in morkConfig, invalid mork db

RESOLVED FIXED

Status

MailNews Core
Database
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Bienvenu, Assigned: Bienvenu)

Tracking

({fixed1.8.1.3, fixed1.8.1.4})

Trunk
x86
Windows XP
fixed1.8.1.3, fixed1.8.1.4

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

10 years ago
I've started to see an actual mork db corruption, caused by an actual mork bug - at least, I think it's a mork bug. The situation usually arises when we have a saved search on a folder, and we make a change to a message that results in the message no longer belonging to the saved search. We cut the message from the table corresponding to the saved search. Sometimes one or more of the cells in the row is dirty, so we write out the dirty cells. But, we don't write out the dirty atoms for the cells, and in some situations, the cell atoms are dirty. When we try to reload the db, it fails because the atoms for the row are not in any dictionary we've loaded. Most of the corruptions I've seen have to do with flag values that aren't in any dictionary.

Here's an example of a bad bit of db. This is a change transaction...

@$${62{@
[97:m(^93=2)]
{189A611:^80 {(k^B1:c)(s=9u)} -
  [148784D(^88^AE0)]}
<(ADF=21)(AE1=45dddb35)>[-1:^9F(^AC=1)(^AD=1)(^A4^ADD)(^A1=18e)(^A2=21)
    (^A3^AD9)(^A5^AE1)(^88=4)(^B9^AD8)(^BB=12)(^BC=2)(^BD=0)(^BE=0)
    (^C1=0)(^C0=0)(^C9^AA6)]
@$$}62}@

In particular, we're cutting the row 48784D from the saved search, in this snippet here:

{189A611:^80 {(k^B1:c)(s=9u)} -
  [148784D(^88^AE0)]}

But, the flags cell (88) is dirty, so we're writing out the dirty cell as well. But the cell value is an atom, AE0, that we've never written out in any dictionary. 

I've changed the code so that if we're writing a row, and some of the cells have  dirty atoms, we write out the dirty atoms in a dictionary before the row cell changes.

I'm not sure why this isn't usually more of a problem in mork - I suspect it's because the atoms usually end up in a dictionary long before the cut happens.

I'll attach a patch in a second. I'm still debugging this, to see if it fixes the problem, and indeed, to see if I ever hit the code I've added.
(Assignee)

Comment 1

10 years ago
Created attachment 256409 [details] [diff] [review]
possible fix
(Assignee)

Updated

10 years ago
Attachment #256409 - Attachment is patch: true
(Assignee)

Comment 2

10 years ago
Comment on attachment 256409 [details] [diff] [review]
possible fix

this doesn't work - the dictionary has to be written out earlier.
Attachment #256409 - Attachment is obsolete: true
(Assignee)

Comment 3

10 years ago
Created attachment 256655 [details] [diff] [review]
a different approach

this might work better - my thinking is that we have rows in the change list that are not in the table, so their dirty atoms don't get written out in the normal table dict writing pass. This patch makes it so we also look for dirty atoms in the change list rows, and write them, if any.

The Tell calls and the assertion are just for me to see if we ever write out any data from the new code.

I left the previous change in this patch - it's an easy way to see if this code doesn't work, because I'll hit a breakpoint in that code.
(Assignee)

Comment 4

10 years ago
Created attachment 256790 [details] [diff] [review]
proposed fix

OK, this fix seems to work - I ran into a situation where the new code did write out some dirty items in the dictionary, and I was able to load the DB afterwards.

While it's scary to change Mork at this late date, this code only writes out data in relatively rare situations, and prior to this patch, in those situations, we'd have a corrupt db, so it's hard for this patch to make things worse. Plus, it seems to work.
Attachment #256655 - Attachment is obsolete: true
Attachment #256790 - Flags: superreview?(mscott)

Updated

10 years ago
Attachment #256790 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 5

10 years ago
Comment on attachment 256790 [details] [diff] [review]
proposed fix

we need this for TB 2.0
Attachment #256790 - Flags: approval1.8.1.3?
Comment on attachment 256790 [details] [diff] [review]
proposed fix

approved for 1.8 branch, a=dveditz for drivers
Attachment #256790 - Flags: approval1.8.1.3? → approval1.8.1.3+
(Assignee)

Comment 7

10 years ago
fixed on trunk and branch.

This one will be pretty difficult for QA to verify
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Keywords: fixed1.8.1.3
Resolution: --- → FIXED
adding fixed keyword to match renamed approval flag
Keywords: fixed1.8.1.4
David: Do you have a suggestion as to how we can verify this on the QA side?
(Assignee)

Comment 10

10 years ago
Marcia, no - it's very hard to verify. I know I haven't seen any of the corruptions that I was seeing on a semi-regular basis.
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.