crash [@ nsMsgDBFolder::OnMessageClassified via bayesian analysis, with aMsgURI null?
Categories
(MailNews Core :: Database, defect)
Tracking
(thunderbird_esr68 fixed)
Tracking | Status | |
---|---|---|
thunderbird_esr68 | --- | fixed |
People
(Reporter: wsmwk, Assigned: benc)
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
1.92 KB,
patch
|
mkmelin
:
review+
wsmwk
:
approval-comm-esr68+
|
Details | Diff | Splinter Review |
Updated•14 years ago
|
Reporter | ||
Comment 2•13 years ago
|
||
Reporter | ||
Comment 3•7 years ago
|
||
Reporter | ||
Comment 4•5 years ago
•
|
||
Interesting that many users only crash once (per crash stats).
bp-432a3657-15a4-41f8-916b-006480200105 is a current example that appears to have no additional addons. Can you take a crack using timeless' analysis
Assignee | ||
Comment 5•5 years ago
|
||
Offending line:
https://hg.mozilla.org/releases/comm-esr68/annotate/dfeab0e6d5a43c6bcb1575bbdc5c0d018b42df9b/mailnews/base/util/nsMsgDBFolder.cpp#l2215
as per comment #1, it looks like it's occurring when MessageClassifier::classifyNextMessage
is trying to signal the end of a batch. It passes in a null aMsgURI
to OnClassifyMessage()
to indicate it's done.
Then as part of the end-of-batch processing, nsMsgDBFolder::OnClassifyMessage()
is dying because it's mDatabase
is null (I think)...
Stack traces show it happens via both nsMsgLocalMailFolder::OnClassifyMessage()
and nsImapMailFolder::OnClassifiyMessage()
... so doesn't look protocol-specific.
Assignee | ||
Comment 6•5 years ago
|
||
OK, one idea. mDatabase is (kind of) lazy-loaded. So this patch adds a GetDatabase() call to the start of OnMessageClassified(), to ensure mDatabase is set.
No way to test it without a test case, but it seems like a plausible fix...
Assignee | ||
Comment 7•5 years ago
|
||
Another thought:
There are a few operations in nsMsgDBFolder that open the database if it's not already open and close it again afterwards (nulling mDatabase). If it was already open they just leave it as is.
So it could be that the database was closed during message classification, but that wouldn't necessarily stop these kinds of operations from succeeding. And then the end-of-batch handling would fail because the db was still closed. Either way, my patch should handle that. But maybe my patch should be more aggressive about closing the database afterwards? I really don't know...
Comment 8•5 years ago
|
||
Updated•5 years ago
|
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/2b1818522523
Ensure msg db is available in nsMsgDBFolder::OnMessageClassified(). r=mkmelin
Reporter | ||
Comment 10•5 years ago
|
||
The crash rate is low enough we can just let this ride until it hits beta 74 which is a couple weeks away. Then uplift to 68.
Reporter | ||
Comment 11•5 years ago
•
|
||
(amended comment)
One beta 73 crash bp-68776f29-a366-4ca0-a40c-6bcd10200228 in the past month.
Comment 12•5 years ago
|
||
That crash is 73.0b0 so wouldn't have this fix so I'd say it's gone.
Reporter | ||
Comment 13•5 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #12)
That crash is 73.0b0 so wouldn't have this fix so I'd say it's gone.
Agreed - thanks for the correction. However, statistically the crash rate is too low in beta to declare it is gone.
That said, I think we should take the patch on esr
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Comment 14•5 years ago
|
||
bugherder uplift |
Thunderbird 68.6.0:
https://hg.mozilla.org/releases/comm-esr68/rev/4547876868f2
Reporter | ||
Updated•5 years ago
|
Description
•