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

RESOLVED FIXED

Status

()

Core
Networking
P2
normal
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: mcmanus)

Tracking

({assertion, fixed1.9.1})

Trunk
x86
Mac OS X
assertion, fixed1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
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)
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.
Blocks: 464838
Flags: blocking1.9.1?
Assignee: nobody → mcmanus
(Assignee)

Comment 2

9 years ago
hmm. ok, i'm on it.
(Assignee)

Comment 3

9 years ago
Created attachment 356518 [details] [diff] [review]
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.
Attachment #356518 - Flags: review?(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 on attachment 356518 [details] [diff] [review]
create a ref counted listener class safe to run on non-main thread

Looks great.
Attachment #356518 - Flags: superreview+
Attachment #356518 - Flags: review?(bzbarsky)
Attachment #356518 - Flags: review+

Updated

9 years ago
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Pushed http://hg.mozilla.org/mozilla-central/rev/0a7c32d52f89 as a diff on top of bug 464838.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a0d75a7e0719 on 1.9.1.
Keywords: fixed1.9.1
Depends on: 473974
You need to log in before you can comment on or make changes to this bug.