Crash in [@ nsAbQueryLDAPMessageListener::OnLDAPError]
Categories
(Thunderbird :: General, defect)
Tracking
(thunderbird_esr78+ fixed, thunderbird83 wontfix)
People
(Reporter: wsmwk, Assigned: benc)
References
(Regression)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
(deleted),
patch
|
mkmelin
:
review+
wsmwk
:
approval-comm-esr78+
|
Details | Diff | Splinter Review |
This crash exists on 83 beta, despite the recent fixing of Bug 1673205 in 83.0b2.
Crash report: https://crash-stats.mozilla.org/report/index/8d283bf1-64e1-4652-94cc-a2c290201110
Reason: EXC_BAD_ACCESS / EXC_I386_GPFLT
Top 10 frames of crashing thread:
0 XUL nsAbQueryLDAPMessageListener::OnLDAPError comm/mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp:104
1 XUL mozilla::detail::RunnableFunction<nsLDAPConnection::InvokeErrorCallback xpcom/threads/nsThreadUtils.h:577
2 XUL mozilla::RunnableTask::Run xpcom/threads/TaskController.cpp:245
3 XUL mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:515
4 XUL mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:374
5 XUL mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal xpcom/threads/nsThreadUtils.h:577
6 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1197
7 CoreFoundation -[__NSArrayM removeObjectsInRange:]
8 XUL NS_ProcessPendingEvents xpcom/threads/nsThreadUtils.cpp:461
9 XUL nsBaseAppShell::NativeEventCallback widget/nsBaseAppShell.cpp:87
Assignee | ||
Comment 1•4 years ago
|
||
Hmm. I'm a bit baffled by this one...
Happens on Linux, Windows and OSX in a pretty consistent-looking form.
I'd say it came in with Bug 1659947 (which is where I added the OnLDAPError method to the listener).
101 NS_IMETHODIMP nsAbQueryLDAPMessageListener::OnLDAPError(nsresult status,
102 nsISupports* secInfo) {
103 if (mResultListener) {
104 mResultListener->OnQueryResult(
105 nsIAbDirectoryQueryResultListener::queryResultError,
106 nsILDAPErrors::OTHER);
107 }
108 return NS_OK;
109 }
It's crashing on line 104, trying to read location 0x0, but mResultListener is checked in line 103.
The only thing I can think of is that mResultListener (or even the "this" ptr) is being cleared on another thread between lines 103 and 104...
We're already on the main thread at this point, and I thought all the core directory query stuff was single-threaded?
And in any case, there doesn't seem to be any code anywhere that clears mResultListener (it's set in the ctor and lasts for the whole lifespan of its owner).
The calling code is here (running on the socket transport thread):
https://searchfox.org/comm-central/source/ldap/xpcom/src/nsLDAPConnection.cpp#472
NS_DispatchToMainThread(NS_NewRunnableFunction(
"InvokeErrorCallback", [listener, status, tsi, location]() {
listener->OnLDAPError(status, tsi, location);
}));
All the variables are copied into the closure so there shouldn't be any scoping issues there. listener and tsi are nsCOMPtr<>, so the closure will be getting a reference to hold them in scope... listener
provides the "this" pointer in OnLDAPError(), so the nsCOMPtr<> reference should hold it in existance just fine.
Without a way to replicate this, I'm kind of out of ideas...
Assignee | ||
Comment 2•4 years ago
|
||
I doubt this will fix it, but my understanding of C++ closures could be all wrong, so who knows :-)
In my understanding this should be exactly the same, but the patch version is probably a slightly nicer syntax anyway...
Comment 3•4 years ago
|
||
Updated•4 years ago
|
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/1a56d35ed594
Trivial simplification to nsLDAPConnection::InvokeErrorCallback() implementation. r=mkmelin
Assignee | ||
Comment 5•4 years ago
|
||
I feel RESOLVED/FIXED might be a little optimistic here, but hey, we live in hope ;-)
Reporter | ||
Comment 6•4 years ago
|
||
So far NO crashes since 2020110820332, for example bp-b17fe55f-47db-4f8a-b70c-8ec7f0201117
But I think we need to see what happens on beta this week end next.
Assignee | ||
Comment 7•4 years ago
|
||
I'm half hoping that this crash does show up again.
If it doesn't, it's off to dull-C++-in-depth-pedantic-detail-land for me!
Reporter | ||
Comment 8•4 years ago
|
||
Updated•4 years ago
|
Comment 9•4 years ago
|
||
Comment on attachment 9187044 [details] [diff] [review]
1676503-simplify-InvokeErrorCallback-1.patch
[Approval Request Comment]
Regression caused by (bug #): bug 1659947 which went to 78.5.1
Crash fix.
Reporter | ||
Comment 10•4 years ago
|
||
Comment on attachment 9187044 [details] [diff] [review]
1676503-simplify-InvokeErrorCallback-1.patch
[Triage Comment]
Approved for esr78
Assignee | ||
Comment 11•4 years ago
|
||
Sighs, and goes to read up on the intricacies of fancy new C++ features. Again.
Comment 12•4 years ago
|
||
bugherder uplift |
Thunderbird 78.6.0:
https://hg.mozilla.org/releases/comm-esr78/rev/b1e98ace50e1
Reporter | ||
Comment 13•4 years ago
|
||
Not possible to verify fixed on 78.6.0 - no crashes ever seen for this signature on release channel.
But clearly gone in beta.
Reporter | ||
Updated•4 years ago
|
Description
•