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


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

Description Kyle Huey [:khuey] (khuey@mozilla.com) 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::~PendingPACQuery+0x0000000000000028
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
D
xul!`anonymous namespace'::DNSListenerProxy::`scalar deleting destructor'+0x0000
00000000000F
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)
MSVCR100D!beginthreadex+0x0000000000000243
MSVCR100D!beginthreadex+0x00000000000001D4
kernel32!BaseThreadInitThunk+0x0000000000000012
ntdll!RtlInitializeExceptionChain+0x0000000000000063
ntdll!RtlInitializeExceptionChain+0x0000000000000036

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] (khuey@mozilla.com) 2011-12-14 10:02:58 PST
The case that's a regression from Bug 675221.
Comment 3 Benjamin Smedberg [: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

thanks!
Comment 5 Benjamin Smedberg [:bsmedberg] 2011-12-30 10:06:03 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/fca43c453bc4
Comment 6 Phil Ringnalda (:philor) 2011-12-31 19:49:46 PST
https://hg.mozilla.org/mozilla-central/rev/fca43c453bc4
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 [: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.