crash [@ nsMsgQuickSearchDBView::OnSearchDone ] - .. - [@ nsMsgSearchSession::InterruptSearch] when change folders before search is finished

RESOLVED FIXED in Thunderbird 19.0

Status

MailNews Core
Backend
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: NoOp, Assigned: rkent)

Tracking

({crash})

Trunk
Thunderbird 19.0
crash
Bug Flags:
in-testsuite -

Thunderbird Tracking Flags

(thunderbird17 fixed, thunderbird18 fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Linux i686; rv:8.0) Gecko/20111110 Firefox/8.0 SeaMonkey/2.5
Build ID: 20111110143639

Steps to reproduce:

I have two folders set up to capture searches from an nntp list via gmane.org: 
gmane.comp.documentfoundation.libreoffice.user
The search searches the message body text for LibreOffice versions; 3.3. and 3.4. respectively. The resulting searchs are listed in my email folder as:
Documentfoundation
  - 3.3.4
  - 3.4.3



Actual results:

clickin on on of the folders (3.3.4 or 3.4.3) causes SeaMonkey to crash. However it is not consistent & doesn't happen every time.


Expected results:

Not crash.
Crash reports are located here:
<https://crash-stats.mozilla.com/report/index/bp-67259d78-f9bf-4e80-9c6d-364322111025>
<https://crash-stats.mozilla.com/report/index/bp-da5e1789-1e5f-43e2-8a56-9f5832111111>

Advise on the mozilla.dev.apps.seamonkey (Neil) is to include reference to:
https://bugzilla.mozilla.org/show_bug.cgi?id=545955
[(filterbar) quick search filtering outside of full searching (implement filter bar, separate quickfilter and global search)] as he believe the code in that bug report may be a contributing factor.
(Reporter)

Comment 1

6 years ago
CC'ing asuth (patch author) and :bienvenu (reviewer) per Neil's recommendation.

Updated

6 years ago
Crash Signature: [@ nsMsgQuickSearchDBView::OnSearchDone ]
Keywords: crash, crashreportid
Summary: [@ nsMsgQuickSearchDBView::OnSearchDone ] → crash [@ nsMsgQuickSearchDBView::OnSearchDone ]

Comment 2

6 years ago
I've got some of those under Windows, too (rare, but going on for quite a while):

<https://crash-stats.mozilla.com/report/index/bp-88b116a0-1293-45cf-a205-8f7222111022>
<https://crash-stats.mozilla.com/report/index/bp-3f118691-2dc8-4e41-af7a-015812110829>
<https://crash-stats.mozilla.com/report/index/bp-a288b26f-e4b9-446d-99b2-c1cbc2110718>

It usually happens, if I change folders while the search hasn't finished yet.
(Reporter)

Comment 3

6 years ago
Ah interesting. Thanks for that. If I recall that seems to have been what I had done as well (change folders while the search hasn't finished). I'll give it a try again today when I'm ready to close down SeaMonkey.
(Reporter)

Comment 4

6 years ago
That did the trick... changed folders while the search hadn't finished.
<https://crash-stats.mozilla.com/report/index/bp-51416062-c2f1-4173-ad93-275632111117>

Updated

6 years ago
Severity: normal → critical
bp-d4d3cb0a-ddbc-48d4-94ce-aa3652120217
Status: UNCONFIRMED → NEW
Crash Signature: [@ nsMsgQuickSearchDBView::OnSearchDone ] → [@ nsMsgQuickSearchDBView::OnSearchDone ] [@ nsMsgQuickSearchDBView::OnSearchDone(unsigned int)]
Component: General → Backend
Ever confirmed: true
Keywords: crashreportid
Product: SeaMonkey → MailNews Core
QA Contact: general → backend
Summary: crash [@ nsMsgQuickSearchDBView::OnSearchDone ] → crash [@ nsMsgQuickSearchDBView::OnSearchDone ] - .. - [@ nsMsgSearchSession::InterruptSearch] when change folders before search is finished
Version: SeaMonkey 2.5 Branch → Trunk
SeaMonkey (Linux64) bug 793447 has the same signature. Related?
(Assignee)

Comment 7

5 years ago
A call to nsMsgDBView::Close nulls m_db, yet the code at the crashing address assumes that m_db is valid, and crashes if not.

Seems pretty clear that all methods in the search views need to behave reasonably with null m_db. I think that the correct thing to do here, as I am starting to do in other places, is to acquire a local reference to the db from the folder and use that, else just return.
(Assignee)

Updated

5 years ago
Assignee: nobody → kent
Status: NEW → ASSIGNED
Duplicate of this bug: 793447
(Assignee)

Comment 9

5 years ago
Created attachment 663926 [details] [diff] [review]
check m_db at the start

It seems to me that if the view has been closed so that m_db is null, there is no point in going through the bulk of this method, so we should just quit there.

I'm not sure if a Close() can happen while executing this method, but I don't think so, so I don't need to check m_db later at the crash point.
Attachment #663926 - Flags: review?(mozilla)
(Assignee)

Updated

5 years ago
Attachment #663926 - Flags: review?(mozilla) → review?(irving)
Comment on attachment 663926 [details] [diff] [review]
check m_db at the start

Review of attachment 663926 [details] [diff] [review]:
-----------------------------------------------------------------

nsMsgDBView::Close() and nsMsgQuickSearchDBView::OnSearchDone() are both called on the UI thread, as far as I can tell, so we don't need to worry about synchronization.
Attachment #663926 - Flags: review?(irving) → review+
(Assignee)

Comment 11

5 years ago
Comment on attachment 663926 [details] [diff] [review]
check m_db at the start

Checked in http://hg.mozilla.org/comm-central/rev/9448de242754
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
OS: Linux → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 19.0
(Assignee)

Comment 12

5 years ago
Comment on attachment 663926 [details] [diff] [review]
check m_db at the start

This patch should land in tb-17 after baking in core.


User impact if declined: crash

Testing completed (on c-c, etc.): checked in, still needs baking

Risk to taking this patch (and alternatives if risky): None.
Attachment #663926 - Flags: approval-comm-beta?
Comment on attachment 663926 [details] [diff] [review]
check m_db at the start

[Triage Comment]
Yep, lets get this onto branches
Attachment #663926 - Flags: approval-comm-beta?
Attachment #663926 - Flags: approval-comm-beta+
Attachment #663926 - Flags: approval-comm-aurora+
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/d271de33102d
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/7dbb40e04465
status-thunderbird17: --- → fixed
status-thunderbird18: --- → fixed
(Reporter)

Comment 15

5 years ago
Thanks for the fix. Can we please also ensure that this fix is included in SeaMonkey 14? Thanks.
(In reply to NoOp from comment #15)
> Thanks for the fix. Can we please also ensure that this fix is included in
> SeaMonkey 14? Thanks.

The fix landed on comm-beta after SM 2.14b3 was tagged, so it'll automatically be included in SM 2.14b4 (unless it's backed out before then).
You need to log in before you can comment on or make changes to this bug.