451127 - "ASSERTION: nsSimpleURI not thread-safe" through nsInputStreamTransport destructor

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Jesse Ruderman]

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

nsSimpleURI::Internal::Release() (nsUnicharUtils.cpp:)
nsSimpleURI::Release() (nsUnicharUtils.cpp:)
nsCOMPtr<nsIURI>::~nsCOMPtr() (nsUnicharUtils.cpp:)
nsCOMPtr<nsIURI>::~nsCOMPtr() (nsUnicharUtils.cpp:)
nsJSThunk::~nsJSThunk() (nsUnicharUtils.cpp:)
nsJSThunk::Release() (nsUnicharUtils.cpp:)
nsCOMPtr<nsIInputStream>::~nsCOMPtr() (nsUnicharUtils.cpp:)
nsCOMPtr<nsIInputStream>::~nsCOMPtr() (nsUnicharUtils.cpp:)
nsInputStreamTransport::~nsInputStreamTransport() (nsUnicharUtils.cpp:)
nsInputStreamTransport::Release() (nsUnicharUtils.cpp:)
nsCOMPtr<nsIInputStream>::assign_assuming_AddRef(nsIInputStream*) (pldhash.c:)
nsCOMPtr<nsIInputStream>::assign_with_AddRef(nsISupports*) (pldhash.c:)
nsCOMPtr<nsIInputStream>::operator=(nsIInputStream*) (pldhash.c:)
nsAStreamCopier::Process() (pldhash.c:)
nsAStreamCopier::Run() (pldhash.c:)
nsThreadPool::Run() (pldhash.c:)
nsThread::ProcessNextEvent(int, int*) (pldhash.c:)
NS_ProcessNextEvent_P(nsIThread*, int) (pldhash.c:)
nsThread::ThreadFunc(void*) (pldhash.c:)
_pt_root (<command line>:47)
_pthread_start (/usr/lib/libSystem.B.dylib)
thread_start (/usr/lib/libSystem.B.dylib)

Related to bug 436341 and/or bug 450914?

Comment 1

[Shawn Wilsher :sdwilsh]

not related to bug 450914

Comment 2

[Shawn Wilsher :sdwilsh]

if we got steps to reproduce here, it'd be awesome.  Need to figure out who is creating the URI in order to do anything.

Comment 3

[Jesse Ruderman]

I don't think I can do that.  Should I mark this bug report as incomplete?  If I manage to come up with steps later, I'll file a new bug and CC you :)

Comment 4

[Shawn Wilsher :sdwilsh]

Yeah, let's go with INCOMPLETE then.

Comment 5

[timeless]

Attachment: see thunk patch for bug 400322 for an explanataion of sorts

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 336621 [details] [diff] [review]
see thunk patch for bug 400322 for an explanataion of sorts

>-    // Get the script string to evaluate...
>-    nsCAutoString script;
>-    nsresult rv = mURI->GetPath(script);
>-    if (NS_FAILED(rv)) return rv;
>+    NS_ENSURE_STATE(!mScript.IsEmpty());

Why are you adding that NS_ENSURE_STATE?  We didn't use to do any such thing, and I don't think we should be adding it.

>+    NS_ENSURE_STATE(!mURL.IsEmpty());

Same here.

r=bzbarsky with those two lines removed.

Comment 7

[Phil Ringnalda (:philor)]

timeless: you have review comments in comment 6: please attach a patch addressing them, and ask for sr on it.

Comment 8

[timeless]

Attachment: per bz

Comment 9

[Phil Ringnalda (:philor)]

Comment on attachment 355319 [details] [diff] [review]
per bz

Personally, I can't wait for the new policy making it the reviewer's call whether something needs sr, rather than the receiver in a case of patch bankruptcy having to figure it out.

Comment 10

[timeless]

Attachment: per bz

Comment 11

[Phil Ringnalda (:philor)]

Comment on attachment 355325 [details] [diff] [review]
per bz

Sigh.

Comment 12

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/b087a09f82aa (including the nsresult rv; that somehow snuck out of the second "per bz").