473089 - "ASSERTION: nsStandardURL not thread-safe" due to nsDNSAsyncRequest and nsHTMLDNSPrefetch

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[Jesse Ruderman]

I can't reproduce, but I got a stack trace when I hit this.

###!!! ASSERTION: nsStandardURL not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /Users/jruderman/central/netwerk/base/src/nsStandardURL.cpp, line 872

nsStandardURL::Release()
nsCOMPtr<nsIURI>::assign_assuming_AddRef(nsIURI*)
nsCOMPtr<nsIURI>::assign_with_AddRef(nsISupports*)
nsCOMPtr<nsIURI>::operator=(nsIURI*)
nsHTMLDNSPrefetch::nsDeferrals::~nsDeferrals()
nsHTMLDNSPrefetch::nsDeferrals::Release()
nsCOMPtr<nsIDNSListener>::assign_assuming_AddRef(nsIDNSListener*)
nsCOMPtr<nsIDNSListener>::assign_with_AddRef(nsISupports*)
nsCOMPtr<nsIDNSListener>::operator=(nsIDNSListener*)
nsDNSAsyncRequest::OnLookupComplete(nsHostResolver*, nsHostRecord*, unsigned int)
nsHostResolver::OnLookupComplete(nsHostRecord*, unsigned int, PRAddrInfo*)
nsHostResolver::ThreadFunc(void*)
_pt_root (/Users/jruderman/central/nsprpub/pr/src/md/unix/os_Darwin.s:47)
_pthread_start (/usr/lib/libSystem.B.dylib)
thread_start (/usr/lib/libSystem.B.dylib)

Comment 1

[Boris Zbarsky [:bzbarsky]]

Um, yeah.

We can't allow the deferrals destructor to run on a random thread.  We especially can't allow that if it's holding refs to elements (see bug 464838) as opposed to URIs.  URIs aren't threadsafe but will sorta do ok in this case, whereas elements might well confuse the cycle collector badly.

I wonder whether we can just proxy the release to the main thread or something... Either that, or use a dns listener which is distinct from the deferrals object, and whose destruction will not entail releasing anything else.  The latter seems like the way to go to me, for simplicity.

Patrick, can you create a patch to that ASAP?  This sort of blocks landing bug 464838.

Comment 2

[Patrick McManus [:mcmanus]]

hmm. ok, i'm on it.

Comment 3

[Patrick McManus [:mcmanus]]

Attachment: create a ref counted listener class safe to run on non-main thread

I'm attaching this here for review by itself, but it won't apply because of deps with the other 464838 changes. I will respin 464838 in order to contain this code.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I'm pretty happy with a diff either on top of 464838 or preceding it.  I'll just land them as two separate changesets.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 356518 [details] [diff] [review]
create a ref counted listener class safe to run on non-main thread

Looks great.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/0a7c32d52f89 as a diff on top of bug 464838.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a0d75a7e0719 on 1.9.1.