Closed
Bug 1382380
Opened 7 years ago
Closed 7 years ago
assertion failure: ScriptLoadHandler not thread-safe
Categories
(Core :: Networking: HTTP, defect)
Core
Networking: HTTP
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: mayhemer, Assigned: schien)
Details
(Whiteboard: [necko-active])
Attachments
(1 file)
(not sure I haven't reported this earlier already...)
today m-c, win, happened during shutdown:
the thread where the release happens doesn't have a name (a naming should be added!)
the mThread member seems to point to the main thread.
> mozglue.dll!MOZ_CrashOOL(0x193ba528, 43, 0x1b581fb8) Line 33 C++
xul.dll!nsAutoOwningThread::AssertCurrentThreadOwnsMe(0x1b581fb8) Line 45 C++
xul.dll!nsAutoOwningThread::AssertOwnership<34>(...) Line 70 C++
xul.dll!mozilla::dom::ScriptLoadHandler::Release() Line 41 C++
xul.dll!nsCOMPtr<nsIIncrementalStreamLoaderObserver>::~nsCOMPtr<nsIIncrementalStreamLoaderObserver>() Line 406 C++
xul.dll!nsIncrementalStreamLoader::~nsIncrementalStreamLoader() Line 21 C++
xul.dll!nsIncrementalStreamLoader::`scalar deleting destructor'() C++
xul.dll!nsIncrementalStreamLoader::Release() Line 45 C++
xul.dll!nsCOMPtr<nsIStreamListener>::~nsCOMPtr<nsIStreamListener>() Line 406 C++
xul.dll!mozilla::net::HttpBaseChannel::~HttpBaseChannel() Line 239 C++
xul.dll!mozilla::net::HttpChannelChild::~HttpChannelChild() Line 207 C++
xul.dll!mozilla::net::HttpChannelChild::`scalar deleting destructor'() C++
xul.dll!mozilla::net::HttpChannelChild::Release() Line 249 C++
xul.dll!mozilla::RefPtrTraits<mozilla::net::HttpChannelChild>::Release(0x29d4d000) Line 41 C++
xul.dll!RefPtr<mozilla::net::HttpChannelChild>::ConstRemovingRefPtrTraits<mozilla::net::HttpChannelChild>::Release(0x29d4d000) Line 395 C++
xul.dll!RefPtr<mozilla::net::HttpChannelChild>::~RefPtr<mozilla::net::HttpChannelChild>() Line 78 C++
xul.dll!mozilla::net::HttpBackgroundChannelChild::ActorDestroy(Deletion) Line 470 C++
xul.dll!mozilla::net::PHttpBackgroundChannelChild::DestroySubtree(Deletion) Line 418 C++
xul.dll!mozilla::net::PHttpBackgroundChannelChild::OnMessageReceived({...}) Line 371 C++
xul.dll!mozilla::ipc::PBackgroundChild::OnMessageReceived({...}) Line 1608 C++
xul.dll!mozilla::ipc::MessageChannel::DispatchAsyncMessage({...}) Line 2093 C++
xul.dll!mozilla::ipc::MessageChannel::DispatchMessageW({...}) Line 2021 C++
xul.dll!mozilla::ipc::MessageChannel::RunMessage({...}) Line 1889 C++
xul.dll!mozilla::ipc::MessageChannel::MessageTask::Run() Line 1922 C++
xul.dll!nsThread::ProcessNextEvent(true, 0x1e6ff171) Line 1447 C++
xul.dll!NS_ProcessNextEvent(0x01784240, true) Line 489 C++
xul.dll!mozilla::net::nsSocketTransportService::Run() Line 976 C++
xul.dll!nsThread::ProcessNextEvent(false, 0x1e6ffa45) Line 1447 C++
xul.dll!NS_ProcessNextEvent(0x01784240, false) Line 489 C++
xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(0x0313e480) Line 339 C++
xul.dll!MessageLoop::RunInternal() Line 321 C++
xul.dll!MessageLoop::RunHandler() Line 314 C++
xul.dll!MessageLoop::Run() Line 294 C++
xul.dll!nsThread::ThreadFunc(0x00cfda14) Line 508 C++
nss3.dll!_PR_NativeRunThread(0x01751920) Line 397 C
nss3.dll!pr_root(0x01751920) Line 95 C
ucrtbase.dll!__set_app_type() Unknown
kernel32.dll!@BaseThreadInitThunk@12() Unknown
mozglue.dll!patched_BaseThreadInitThunk(0, 0x758ae840, 0x02f2b7f8) Line 816 C++
ntdll.dll!__RtlUserThreadStart() Unknown
ntdll.dll!__RtlUserThreadStart@8() Unknown
Flags: needinfo?
Reporter | ||
Comment 1•7 years ago
|
||
And I can provide a log - timestamp,sync,nsHttp:5,cache2:5,RequestContext:5
Flags: needinfo?
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(schien)
Assignee | ||
Comment 2•7 years ago
|
||
From the crash stack, It was happened on socket transport thread.
> xul.dll!mozilla::net::nsSocketTransportService::Run() Line 976 C++
The main logic flaw is that we don't know whether the listener supports OMT or not. One thing we can do is to ensure mListener is released on main thread if RetargetDeliveryTo is never called.
Flags: needinfo?(schien)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → schien
Reporter | ||
Comment 3•7 years ago
|
||
(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #2) > From the crash stack, It was happened on socket transport thread. > > xul.dll!mozilla::net::nsSocketTransportService::Run() Line 976 C++ > > The main logic flaw is that we don't know whether the listener supports OMT > or not. One thing we can do is to ensure mListener is released on main > thread if RetargetDeliveryTo is never called. Hmm... maybe the simplest solution is to always release mListener on the main thread (add it to the list in HttpBaseChannel::ReleaseMainThreadOnlyReferences()). We could save small amount of main thread time when we know it's safe to release on any thread, tho. But only if we have that information available 100% reliably - if, as you said, we called RetargetDeliveryTo with success _and_ the mListener has not changed since that or has not been added a new listener to the chain since that time. That can IMO never be known for 100%.
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #3) > (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment > #2) > > From the crash stack, It was happened on socket transport thread. > > > xul.dll!mozilla::net::nsSocketTransportService::Run() Line 976 C++ > > > > The main logic flaw is that we don't know whether the listener supports OMT > > or not. One thing we can do is to ensure mListener is released on main > > thread if RetargetDeliveryTo is never called. > > Hmm... maybe the simplest solution is to always release mListener on the > main thread (add it to the list in > HttpBaseChannel::ReleaseMainThreadOnlyReferences()). We could save small > amount of main thread time when we know it's safe to release on any thread, > tho. But only if we have that information available 100% reliably - if, as > you said, we called RetargetDeliveryTo with success _and_ the mListener has > not changed since that or has not been added a new listener to the chain > since that time. That can IMO never be known for 100%. sgtm, I was a bit worried that always release mListener on main thread is overkill.
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Whiteboard: [necko-active]
Assignee | ||
Comment 6•7 years ago
|
||
@dragana, is there anything blocking the review? I'd like to resolve this issue before the merge day.
Flags: needinfo?(dd.mozilla)
Comment 7•7 years ago
|
||
(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #6) > @dragana, is there anything blocking the review? I'd like to resolve this > issue before the merge day. I reviewed this on a train on Friday, and I submitted a review.... maybe I lost network connectivity and therefore the review has not been published... sorry... stupid moving trains....
Flags: needinfo?(dd.mozilla)
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8888618 [details] Bug 1382380 - ensure mListener/mListenerContext/mCompressListener is released on main thread. https://reviewboard.mozilla.org/r/159628/#review166658
Attachment #8888618 -
Flags: review?(dd.mozilla) → review+
Pushed by schien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b01c620827b9 ensure mListener/mListenerContext/mCompressListener is released on main thread. r=dragana
Assignee | ||
Comment 10•7 years ago
|
||
Thanks for the promptly response, @dragana!
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b01c620827b9
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•