Status
()
People
(Reporter: Benjamin Smedberg, Assigned: Benjamin Smedberg)
Tracking
(Blocks: 1 bug)
Firefox Tracking Flags
(Not tracked)
Details
Attachments
(1 attachment)
8.96 KB,
patch
|
jesup
:
review+
|
Details | Diff | Splinter Review |
Currently if a plugin crashes, GMPParent::ActorDestroy re-enters itself through this stack:
> xul.dll!mozilla::gmp::GMPParent::ActorDestroy(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason aWhy) Line 289 C++
xul.dll!mozilla::gmp::PGMPParent::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason why) Line 758 C++
xul.dll!mozilla::gmp::PGMPParent::OnChannelError() Line 625 C++
xul.dll!mozilla::ipc::MessageChannel::NotifyMaybeChannelError() Line 1558 C++
xul.dll!mozilla::ipc::MessageChannel::Close() Line 1660 C++
xul.dll!mozilla::gmp::PGMPParent::Close() Line 94 C++
xul.dll!mozilla::gmp::GMPParent::UnloadProcess() Line 127 C++
xul.dll!mozilla::gmp::GMPParent::ActorDestroy(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason aWhy) Line 298 C++
xul.dll!mozilla::gmp::PGMPParent::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason why) Line 758 C++
xul.dll!mozilla::gmp::PGMPParent::OnChannelError() Line 625 C++
xul.dll!mozilla::ipc::MessageChannel::NotifyMaybeChannelError() Line 1558 C++
xul.dll!mozilla::ipc::MessageChannel::OnNotifyMaybeChannelError() Line 1587 C++
xul.dll!DispatchToMethod<mozilla::ipc::MessageChannel,void (__thiscall mozilla::ipc::MessageChannel::*)(void)>(mozilla::ipc::MessageChannel * obj, void (void) * method, const Tuple0 & arg) Line 384 C++
I don't believe we should call Close from within ActorDestroy, because we already know that the actor is closing. I still need to look at a "normal" shutdown sequence to see if we can skip the Close altogether, or only in some cases.
I'm going to take this, since I'm touching stuff in the area.
(Assignee) | ||
Comment 1•4 years ago
|
||
Created attachment 8458030 [details] [diff] [review] bug1039572-actordestroy-reenter This patch sits on top of bug 1039575: it rejiggers the crash and shutdown sequence so that the state machine of GMPParent is clear and so that ActorDestroy doesn't re-enter.
Attachment #8458030 -
Flags: review?(rjesup)
Comment 2•4 years ago
|
||
Comment on attachment 8458030 [details] [diff] [review] bug1039572-actordestroy-reenter Review of attachment 8458030 [details] [diff] [review]: ----------------------------------------------------------------- Looks quite good! ::: content/media/gmp/GMPParent.cpp @@ +118,5 @@ > +{ > + MOZ_ASSERT(GMPThread() == NS_GetCurrentThread()); > + > + if (mState == GMPStateNotLoaded || mState == GMPStateClosing) { > + MOZ_ASSERT(mVideoDecoders.IsEmpty() && mVideoEncoders.IsEmpty()); cpearce will need to update this for audio, but that will be his patches (in a number of places)
Attachment #8458030 -
Flags: review?(rjesup) → review+
(Assignee) | ||
Comment 3•4 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad1513416811
Target Milestone: --- → mozilla33
(Assignee) | ||
Updated•4 years ago
|
QA Whiteboard: [qa-]
Updated•4 years ago
|
Status: NEW → ASSIGNED
Iteration: --- → 33.3
Comment 4•4 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ad1513416811
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Updated•4 years ago
|
Flags: firefox-backlog+
You need to log in
before you can comment on or make changes to this bug.
Description
•