nsStreamLoader code accessed on secondary thread [ASSERTION: nsStreamLoader not thread-safe]

RESOLVED FIXED

Status

()

P3
normal
RESOLVED FIXED
13 years ago
11 years ago

People

(Reporter: kaie, Assigned: dcamp)

Tracking

({assertion})

Trunk
assertion
Points:
---
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

13 years ago
We occassionaly get a console warning about nsStreamLoader not being thread safe.

We identified the cause to be the destructor of nsIStreamLoaderObserver.

Christian recommended to try NS_ProxyRelease.
While this makes the warning go away, it can introduce a deadlock on application exit, because we need to do synchrounous shutdown of all NSS resources, which requires destroying all our objects first.

The deadlock is caused by the main thread dispatching shutdown events, and the PSM modules's shutdown code trying to proxy a destructor call to the main thread.
QA Contact: psm

Comment 1

12 years ago
Loading http://grml.org/ triggers:

###!!! ASSERTION: nsStreamLoader not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /Users/jruderman/trunk/mozilla/netwerk/base/src/nsStreamLoader.cpp, line 65
Flags: blocking1.9?

Updated

12 years ago
Flags: blocking1.9? → blocking1.9+

Updated

12 years ago
Priority: -- → P2

Comment 2

12 years ago
Is this really only on the branch? Shouldn't block 1.9 if so.
Priority: P2 → P3
(Reporter)

Comment 3

12 years ago
I guess this affects trunk, too, as I'm not aware of relevant changes.
Version: 1.8 Branch → Trunk
Duplicate of this bug: 403693
(In reply to comment #3)
> I guess this affects trunk, too, as I'm not aware of relevant changes.
> 

confirmed on trunk, my steps to reproduce on trunk are :
->Type gmail.google.com into the url bar
-> Before the Gmail UI
(https://www.google.com/accounts/ServiceLogin?service=mail&passive=true ... )is
loaded completely the Assertion occur
Keywords: assertion
Summary: nsStreamLoader code accessed on secondary thread → nsStreamLoader code accessed on secondary thread [ASSERTION: nsStreamLoader not thread-safe]
Loading https://www.bankofamerica.com/ trigger the assertion. (FF Linux)
(Assignee)

Comment 7

11 years ago
(In reply to comment #0)

> Christian recommended to try NS_ProxyRelease.
> While this makes the warning go away, it can introduce a deadlock on
> application exit, because we need to do synchrounous shutdown of all NSS
> resources, which requires destroying all our objects first.
> 
> The deadlock is caused by the main thread dispatching shutdown events, and the
> PSM modules's shutdown code trying to proxy a destructor call to the main
> thread.

Would this actually deadlock?  NS_ProxyRelease proxies the event asynchronously, it wouldn't block the rest of the PSM shutdown.
(Assignee)

Comment 8

11 years ago
Posted patch proxy mListener's release (obsolete) — Splinter Review
... so unless I'm wrong, this should work fine.
Attachment #303166 - Flags: review?(kengert)
(Assignee)

Comment 9

11 years ago
Comment on attachment 303166 [details] [diff] [review]
proxy mListener's release

... and apparently I am, because I just ran into a shutdown deadlock.
Attachment #303166 - Attachment is obsolete: true
Attachment #303166 - Flags: review?(kengert)
(Assignee)

Comment 10

11 years ago
Posted patch v2Splinter Review
so actually the hang I was seeing was caused by bug 417404.  New patch cleans things up a bit.
Assignee: kengert → dcamp
Status: NEW → ASSIGNED
Attachment #303208 - Flags: review?(kengert)
(Assignee)

Updated

11 years ago
Depends on: 417404
(Reporter)

Comment 11

11 years ago
Comment on attachment 303208 [details] [diff] [review]
v2

Using this patch (only), I crash when visiting some SSL sites.
Using this patch combined with the patch from bug 417404, it works and the assertion is gone.

This tells me, we must really wait for 417404 before we can check this in.
Attachment #303208 - Flags: review?(kengert) → review+
(Assignee)

Updated

11 years ago
Attachment #303208 - Flags: superreview?(dveditz)
Comment on attachment 303208 [details] [diff] [review]
v2

sr=dveditz with the caveat about landing 417404 too.
Attachment #303208 - Flags: superreview?(dveditz) → superreview+
(Assignee)

Comment 13

11 years ago
Checking in nsNSSCallbacks.cpp;
/cvsroot/mozilla/security/manager/ssl/src/nsNSSCallbacks.cpp,v  <--  nsNSSCallbacks.cpp
new revision: 1.62; previous revision: 1.61
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.