609585 - crash [@ nsLDAPConnectionLoop::Run()] handling mPendingOperations. [@ nsCOMPtr_base::~nsCOMPtr_base() | nsLDAPConnectionLoop::Run()], [@ libxpcom_core.dylib@0x4011] (Mac)

Status: RESOLVED WORKSFORME

(Directory :: LDAP XPCOM SDK, defect)

Description

[Wayne Mery (:wsmwk)]

crash [@ nsLDAPConnectionLoop::Run()]
~#200 for v3.1.6

bp-9e3ec5db-5c64-48dd-9ee5-8d9432101104 v3.0.5
bp-83cbaf2c-58a4-42dd-8ab0-b23052101014 v3.1.4 "was sending an email"
0	thunderbird.exe	nsLDAPConnectionLoop::Run	directory/xpcom/base/src/nsLDAPConnection.cpp:868
1	xpcom_core.dll	nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:527
2	xpcom_core.dll	NS_ProcessNextEvent_P	objdir-tb/mozilla/xpcom/build/nsThreadUtils.cpp:250
3	xpcom_core.dll	nsThread::ThreadFunc	xpcom/threads/nsThread.cpp:254
4	nspr4.dll	_PR_NativeRunThread	nsprpub/pr/src/threads/combined/pruthr.c:426

Comment 1

[Wayne Mery (:wsmwk)]

same crash, even though different line number??

nsCOMPtr_base::~nsCOMPtr_base() | nsLDAPConnectionLoop::Run()
bp-2bf074a1-bffa-41eb-b988-ffb312101101 v3.1.4
0	xpcom_core.dll	nsCOMPtr_base::~nsCOMPtr_base	objdir-tb/mozilla/xpcom/build/nsCOMPtr.cpp:81
1	thunderbird.exe	nsLDAPConnectionLoop::Run	directory/xpcom/base/src/nsLDAPConnection.cpp:881
2	xpcom_core.dll	nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:527
3	xpcom_core.dll	NS_ProcessNextEvent_P	objdir-tb/mozilla/xpcom/build/nsThreadUtils.cpp:250

Comment 2

[Wayne Mery (:wsmwk)]

not sure there is a Mac version of this crash. but bp-e9a73ab6-1693-43c1-84a9-3568c2101020 @ libxpcom_core.dylib@0x4011 has nsLDAPConnectionLoop::Run on the stack

Comment 3

[timeless]

comment 0 is a null pointer crash:

868 if (mRawConn->mPendingOperations->Count()) { 

mPendingOperations is null.

comment 1 is a different crash (please file a new bug) -- I think comment 2 goes with comment 1.

Comment 4

[timeless]

actually, don't, maybe they're all the same...

Comment 5

[timeless]

actually. I think they're the same. try the following before forking a bug.

Comment 6

[timeless]

Attachment: outline

I haven't asked a compiler what it thinks, but this should outline my thinking.

Comment 7

[Wayne Mery (:wsmwk)]

(In reply to comment #5)
> actually. I think they're the same. try the following before forking a bug.

FWIW, I don't have a testcase, nor a reporter for this. but after checking one month of nsCOMPtr_base::~nsCOMPtr_base() | nsLDAPConnectionLoop::Run() I am waiting to hear back from  bp-41d3f181-42b2-486e-932e-175942101027 (priyank.m) 

And Mac users continue to be more prolific in reporting with their email addresses than other OS. for libxpcom_core.dylib@0x4011
bp-9a2981dc-5564-4ef7-bd78-8d7f62101019 (ken)
bp-a6cda2bb-06ac-4eea-a121-70d2e2101108 (alex)

Comment 8

[David :Bienvenu]

Attachment: patch that compiles but doesn't work.

prev patch didn't compile. This patch compiles, but fails at runtime because nsLDAPConnection::Release grabs the lock before calling delete this, and the destructor calls Close(), which tries also to grab the lock, which generates a PR_ASSERT in the lock code, because we already have the lock. This *may* mean that we don't need the PR_Lock calls in Close, because we can only ever get there from Release().

Comment 9

[David :Bienvenu]

I tried the patch w/o the lock calls in Close, and things seem to work, though there is a thread-safety assertion when the connection gets freed that I don't remember seeing before. I guess I'll attach a patch and see what standard8 thinks...

Comment 10

[David :Bienvenu]

Attachment: tweaked fix

Comment 11

[Mark Banner (:standard8)]

Attachment: The fix

I tweaked this to take account of the recent directory -> ldap move, and to fix a review nit of using // rather than /* ... */

This has r=Standard8

Comment 12

[Wayne Mery (:wsmwk)]

still needs checkin

I have not heard back from priyank.m about bp-41d3f181-42b2-486e-932e-175942101027.
pmed bp-6a6744bc-871c-4926-9029-7ed532101207 (sean)

Comment 13

[Wayne Mery (:wsmwk)]

no version 3.1.7 crashes with this sig in the last 2 weeks - odd?

from comment #12
> pmed bp-6a6744bc-871c-4926-9029-7ed532101207 (sean)
sean doesn't know how his happened.

Alex bp-a6cda2bb-06ac-4eea-a121-70d2e2101108 writes "I am hardly using Thunderbird on Mac, although it is much better than Mac Mail for me, however, the Linux Version of Thunderbird is rock solid, and that's what I am using in daily life though."

Comment 14

[Ed Morley [:emorley]]

This was set checkin-needed 6 months ago, is it still wanted?

Comment 15

[Ludovic Hirlimann [:Usul]]

(In reply to comment #14)
> This was set checkin-needed 6 months ago, is it still wanted?

If it still applies : yes !

Comment 16

[Ed Morley [:emorley]]

applying 609585.txt
patching file ldap/xpcom/src/nsLDAPConnection.cpp
Hunk #1 succeeded at 215 with fuzz 1 (offset -45 lines).
Hunk #2 FAILED at 272
Hunk #3 FAILED at 869

Rebase didn't help.

Seems like bug 630816 and bug 343332 have rotted this considerably - eg the approach here has completely changed:
http://hg.mozilla.org/comm-central/annotate/6a05e36ee576/ldap/xpcom/src/nsLDAPConnection.cpp#l235

Clearing checkin-needed to tidy up the checkin-needed shared search (which was the main reason for comment 14), since this needs more than just a rebase before it can land - and is beyond my expertise.

Comment 17

[Wayne Mery (:wsmwk)]

nsLDAPConnectionLoop::Run() is #152 crash for v3.1.10

Comment 18

[Wayne Mery (:wsmwk)]

All examples on crash-stats are version 3.1.x and 3.0.x