684492 - shutdown crash nsLDAPConnection::~nsLDAPConnection()

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Wayne Mery (:wsmwk)]

shutdown crash nsLDAPConnection::~nsLDAPConnection() 

only appears in v6, and 6.0.1. #67 crash for v6.0.
I don't see what it would have caused the regression.
no apparent correlations.

spot check of 13 crashes - 3 of 13 have nsX509CertValidity::Release on stack, like ...
bp-2048ebc8-acc1-4e98-8f0d-24a622110901 has reporter 	rsimpson
EXCEPTION_ACCESS_VIOLATION_READ
0x0
0	xul.dll	nsLDAPConnection::~nsLDAPConnection	ldap/xpcom/src/nsLDAPConnection.cpp:87
1	xul.dll	nsLDAPConnection::`scalar deleting destructor'	
2	xul.dll	nsLDAPConnection::Release	mailnews/base/src/nsMsgStatusFeedback.cpp:83
3	xul.dll	nsCOMPtr_base::~nsCOMPtr_base	objdir-tb/mozilla/xpcom/build/nsCOMPtr.cpp:81
4	xul.dll	nsAbLDAPDirectoryQuery::~nsAbLDAPDirectoryQuery	mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp:353
5	xul.dll	nsAbLDAPDirectoryQuery::`scalar deleting destructor'	
6	xul.dll	nsXULPrototypeCache::Release	content/xul/document/src/nsXULPrototypeCache.cpp:111
7	xul.dll	nsCOMPtr_base::~nsCOMPtr_base	objdir-tb/mozilla/xpcom/build/nsCOMPtr.cpp:81
8	xul.dll	nsAbLDAPDirectory::~nsAbLDAPDirectory	mailnews/addrbook/src/nsAbLDAPDirectory.cpp:89
9	xul.dll	nsAbLDAPDirectory::`vector deleting destructor'	
10	xul.dll	nsX509CertValidity::Release	js/src/xpconnect/src/xpcstack.cpp:130
11	xul.dll	XPCJSRuntime::GCCallback	js/src/xpconnect/src/xpcjsruntime.cpp:935 


10 of the 13 are like bp-8af7498b-0da9-45d2-972d-ab0d22110902, having nsMsgStatusFeedback::Release on stack
EXCEPTION_ACCESS_VIOLATION_READ
0x0
0	xul.dll	nsLDAPConnection::~nsLDAPConnection	ldap/xpcom/src/nsLDAPConnection.cpp:87
1	xul.dll	nsLDAPConnection::`scalar deleting destructor'	
2	xul.dll	nsMsgStatusFeedback::Release	mailnews/base/src/nsMsgStatusFeedback.cpp:83
3	xul.dll	nsCOMPtr_base::~nsCOMPtr_base	objdir-tb/mozilla/dist/include/nsAutoPtr.h:969
4	xul.dll	nsAbLDAPDirectoryQuery::~nsAbLDAPDirectoryQuery	mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp:353
5	xul.dll	nsAbLDAPDirectoryQuery::`scalar deleting destructor'	
6	xul.dll	nsPKIParamBlock::Release	mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp:344
7	xul.dll	nsCOMPtr_base::~nsCOMPtr_base	objdir-tb/mozilla/dist/include/nsAutoPtr.h:969
8	xul.dll	nsAbLDAPDirectory::~nsAbLDAPDirectory	mailnews/addrbook/src/nsAbLDAPDirectory.cpp:89
9	xul.dll	nsAbLDAPDirectory::`scalar deleting destructor'	
10	xul.dll	nsDocumentCharsetInfo::Release	ldap/xpcom/src/nsLDAPModification.cpp:47

Comment 1

[Makoto Kato [:m_kato]]

When shutdown, observer service may be already released.  We should check whether obsServ is null.

Comment 2

[Makoto Kato [:m_kato]]

Attachment: fix

Comment 3

[Makoto Kato [:m_kato]]

This is a regression by bug 669151

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 567376 [details] [diff] [review]
fix

r=Standard8. Please transplant to aurora and beta as well.

Comment 5

[Mark Banner (:standard8)]

Due to the current closed trunk, and the fact we'd like to ship this in a beta for some testing, I've landed this directly on comm-beta:

http://hg.mozilla.org/releases/comm-beta/rev/7d0d937bdd8a

Setting flag appropriately, we'll still need to land on central and aurora.

Comment 6

[Makoto Kato [:m_kato]]

http://hg.mozilla.org/comm-central/rev/6fae7c213686

Comment 7

[Makoto Kato [:m_kato]]

http://hg.mozilla.org/releases/comm-aurora/rev/fa28ff6e1695

Comment 8

[[e-mail]]

Wow, nsLDAPConnection really likes to violate XPCOM rules:
No passing yourself to other methods in your constructor!
When you're destroyed, it's because nobody has any references to you any more, so it's too late to remove yourself...

Comment 9

[Mark Banner (:standard8)]

(In reply to [e-mail] from comment #8)
> Wow, nsLDAPConnection really likes to violate XPCOM rules:
> No passing yourself to other methods in your constructor!

Err, looks an empty constructor to me: http://hg.mozilla.org/comm-central/file/default/ldap/xpcom/src/nsLDAPConnection.cpp#l72

> When you're destroyed, it's because nobody has any references to you any
> more, so it's too late to remove yourself...

Except nsIObserverService is holding a weak reference, so this is just keeping the list of services clean.