Closed Bug 473089 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: Networking, defect, P2)

x86
macOS
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: mcmanus)

References

Details

(Keywords: assertion, fixed1.9.1)

Attachments

(1 file)

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
hmm. ok, i'm on it.
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+
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
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.