Crash in [@ SearchExtRunnable::~SearchExtRunnable]
Categories
(MailNews Core :: LDAP Integration, defect)
Tracking
(thunderbird_esr68 verified, thunderbird72 fixed, thunderbird73 fixed)
People
(Reporter: wsmwk, Assigned: benc)
References
(Regression)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file, 2 obsolete files)
1011 bytes,
patch
|
jorgk-bmo
:
review+
jorgk-bmo
:
approval-comm-beta+
jorgk-bmo
:
approval-comm-esr68+
|
Details | Diff | Splinter Review |
This crash is new to 68.3.0. Also seen in 71 and 72 beta bp-81cb72dd-2e00-4936-8949-e666d0191204
This bug is for crash report bp-7005e466-331d-4f2a-97df-3fd3e0191204.
Top 10 frames of crashing thread:
0 XUL SearchExtRunnable::~SearchExtRunnable comm/ldap/xpcom/src/nsLDAPOperation.cpp:481
1 XUL SearchExtRunnable::~SearchExtRunnable comm/ldap/xpcom/src/nsLDAPOperation.cpp:475
2 XUL mozilla::Runnable::Release xpcom/threads/nsThreadUtils.cpp:51
3 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1222
4 XUL <name omitted> xpcom/threads/ThreadEventTarget.cpp:163
5 XUL NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486
6 XUL mozilla::net::nsSocketTransportService::Run netwerk/base/nsSocketTransportService2.cpp:1013
7 XUL XUL@0x50b0cf
8 XUL non-virtual thunk to mozilla::net::nsSocketTransportService::Run netwerk/base/nsSocketTransportService2.cpp
9 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1175
Assignee | ||
Comment 1•6 years ago
|
||
Oop, that code has my fingerprints all over it!
I think this patch should fix it...
Comment 2•6 years ago
|
||
Updated•6 years ago
|
Comment 3•6 years ago
|
||
Assignee | ||
Comment 4•6 years ago
|
||
Huh. Turns out that moz_xmalloc()
is infallible after all. It invokes MOZ_CRASH()
in OOM situations and never returns NULL.
Not what I expected, but nice to know!
Patch tweaked accordingly.
(also changed the terminating pointer array entry from 0
to nullptr
while I was there).
Assignee | ||
Updated•6 years ago
|
Reporter | ||
Comment 5•6 years ago
|
||
What is the regressing bug? (I've lost track of it)
Updated•6 years ago
|
Reporter | ||
Updated•6 years ago
|
Updated•6 years ago
|
Comment 6•6 years ago
|
||
Looking at this: Why don't you just add if (!mAttrs) return;
in the DTOR and leave the rest as it is?
Assignee | ||
Comment 7•6 years ago
|
||
I did consider that, but felt it left the code nicer if I removed the special case.
Comment 8•6 years ago
|
||
But why go though the hassle of allocating a one-entry array (which is null anyway) just do de-allocate it later?
Assignee | ||
Comment 9•6 years ago
|
||
(In reply to Jorg K (GMT+1) (PTO to 15th Dec 2019, sporadically reading bugmail) from comment #8)
But why go though the hassle of allocating a one-entry array (which is null anyway) just do de-allocate it later?
Because it removes two special-case conditional paths (one at the allocation, and one in the dtor).
I agree that it's ugly to allocate emptiness, but simplicity over efficiency seemed like a good tradeoff (there's always some idiot who's going to forget to check for null in the dtor ;- )
Anyway - alternate oneliner patch attached, if you prefer it. I'm happy either way as long as my original cock-up gets fixed!
Updated•6 years ago
|
Comment 10•6 years ago
|
||
Updated•6 years ago
|
Updated•6 years ago
|
Comment 11•6 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/bdb4d4a5b9fd
Fix invalid-memory access in LDAP search. r=jorgk
Updated•6 years ago
|
Comment 12•6 years ago
|
||
Uff, I should have commented on the commit message :-(
Comment 13•6 years ago
|
||
Updated•6 years ago
|
Comment 14•6 years ago
|
||
Reporter | ||
Comment 15•6 years ago
|
||
Is this the same issue?
nsLDAPSSLClose bp-1b1dcb21-920c-48f5-8696-9bc730191210
Only seen in 68.3.0
Assignee | ||
Comment 16•6 years ago
|
||
It could definitely be something I screwed up, but I think it's a different issue. Open a new bug for it?
Updated•6 years ago
|
Reporter | ||
Updated•5 years ago
|
Description
•