Last Comment Bug 708935 - Assertion nsStandardURL not threadsafe
: Assertion nsStandardURL not threadsafe
: assertion, regression
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla12
Assigned To: Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg]
Depends on:
Blocks: 675221 710776
  Show dependency treegraph
Reported: 2011-12-08 19:00 PST by Kyle Huey [:khuey] (
Modified: 2012-02-06 15:14 PST (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Proxy potential last-release of nsIDNSListener to the main thread, rev. 1 (2.05 KB, patch)
2011-12-20 06:43 PST, Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg]
mcmanus: review+
akeybl: approval‑mozilla‑aurora-
bugzilla: approval‑mozilla‑beta-
Details | Diff | Splinter Review

Description Kyle Huey [:khuey] ( 2011-12-08 19:00:48 PST
###!!! ASSERTION: nsStandardURL not thread-safe: '_mOwningThread.GetThread() ==
PR_GetCurrentThread()', file c:/dev/mozilla-central/netwerk/base/src/nsStandardURL.cpp, line 966
xul!nsCOMPtr<nsIURI>::~nsCOMPtr<nsIURI>+0x000000000000003B (c:\dev\mozilla-central\obj-i686-pc-mingw32\dist\include\nscomptr.h, line 561)
xul!PendingPACQuery::`scalar deleting destructor'+0x000000000000000F
xul!PendingPACQuery::Release+0x000000000000008A (c:\dev\mozilla-central\netwerk\base\src\nspacman.cpp, line 103)
xul!nsCOMPtr<nsIDNSListener>::~nsCOMPtr<nsIDNSListener>+0x000000000000003B (c:\dev\mozilla-central\obj-i686-pc-mingw32\dist\include\nscomptr.h, line 561)
xul!`anonymous namespace'::DNSListenerProxy::~DNSListenerProxy+0x000000000000001
xul!`anonymous namespace'::DNSListenerProxy::`scalar deleting destructor'+0x0000
xul!`anonymous namespace'::DNSListenerProxy::Release+0x000000000000008D (c:\dev\mozilla-central\netwerk\dns\nsdnsservice2.cpp, line 512)
xul!nsCOMPtr<nsIDNSListener>::assign_assuming_AddRef+0x0000000000000059 (c:\dev\mozilla-central\obj-i686-pc-mingw32\dist\include\nscomptr.h, line 546)
xul!nsCOMPtr<nsIDNSListener>::assign_with_AddRef+0x0000000000000027 (c:\dev\mozilla-central\obj-i686-pc-mingw32\dist\include\nscomptr.h, line 1202)
xul!nsCOMPtr<nsIDNSListener>::operator=+0x0000000000000013 (c:\dev\mozilla-central\obj-i686-pc-mingw32\dist\include\nscomptr.h, line 691)
xul!nsDNSAsyncRequest::OnLookupComplete+0x00000000000000DF (c:\dev\mozilla-central\netwerk\dns\nsdnsservice2.cpp, line 310)
xul!nsHostResolver::OnLookupComplete+0x000000000000025B (c:\dev\mozilla-central\netwerk\dns\nshostresolver.cpp, line 888)
xul!nsHostResolver::ThreadFunc+0x00000000000000CD (c:\dev\mozilla-central\netwerk\dns\nshostresolver.cpp, line 922)
nspr4!_PR_NativeRunThread+0x00000000000000DB (c:\dev\mozilla-central\nsprpub\pr\src\threads\combined\pruthr.c, line 426)
nspr4!pr_root+0x0000000000000019 (c:\dev\mozilla-central\nsprpub\pr\src\md\windows\w95thred.c, line 122)

This is fallout from Bug 675221, I think.
Comment 1 Serge Gautherie (:sgautherie) 2011-12-14 10:01:51 PST
There are other bugs related to this assertion.
Which case is this bug about?
Comment 2 Kyle Huey [:khuey] ( 2011-12-14 10:02:58 PST
The case that's a regression from Bug 675221.
Comment 3 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-12-20 06:43:52 PST
Created attachment 583146 [details] [diff] [review]
Proxy potential last-release of nsIDNSListener to the main thread, rev. 1
Comment 4 Patrick McManus [:mcmanus] 2011-12-20 07:53:22 PST
Comment on attachment 583146 [details] [diff] [review]
Proxy potential last-release of nsIDNSListener to the main thread, rev. 1

Comment 5 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-12-30 10:06:03 PST
Comment 6 Phil Ringnalda (:philor, back in August) 2011-12-31 19:49:46 PST
Comment 7 Serge Gautherie (:sgautherie) 2012-01-11 10:28:12 PST
Comment on attachment 583146 [details] [diff] [review]
Proxy potential last-release of nsIDNSListener to the main thread, rev. 1

[Approval Request Comment]
Regression caused by (bug #): (bug 675221, but) fixes older assertion too.
User impact if declined: (I'm not sure, except) SeaMonkey bug 710776 perma-orange.
Testing completed (on m-c, etc.): comment 6 and (SeaMonkey) bug 710776 comment 2.
Risk to taking this patch (and alternatives if risky):
I assume this is low risk. But I defer to Benjamin to assess whether this patch is applicable/suitable for branches.
Comment 8 Alex Keybl [:akeybl] 2012-01-11 13:43:30 PST
We'll hold in the queue for a risk assessment from Benjamin.
Comment 9 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2012-01-11 16:44:42 PST
This patch is not especially risky but also probably an unusual case: I'd definitely take it on aurora, but would probably take it on beta if there is evidence of crashes in the relevant code (pendingpacquery or dnslistenerproxy signatures).

There appear to be two crash reports on PendingPACQuery which could possibly related, and a few signatures related to async DNS lookups which are present in 9.0.1 and 3.6.*. So I don't know how to interpret that data ;-)
Comment 10 Johnathan Nightingale [:johnath] 2012-01-12 14:41:04 PST
Comment on attachment 583146 [details] [diff] [review]
Proxy potential last-release of nsIDNSListener to the main thread, rev. 1

Discussed in triage - yes for aurora, but feels late in beta to take something whose downside, if we introduced a bug, might be difficult to detect (spread out crash signatures, etc)
Comment 11 Alex Keybl [:akeybl] 2012-02-06 15:13:59 PST
This is fixed on Aurora 12. The same rationale applies for why we wouldn't want to take this on Beta yet. We'll let it ride the trains.

Note You need to log in before you can comment on or make changes to this bug.