crash in mozilla::plugins::PluginAsyncSurrogate::DestroyAsyncStream(_NPStream*)

RESOLVED FIXED in Firefox 38

Status

()

Core
Plug-ins
--
critical
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: mats, Assigned: aklotz)

Tracking

({crash})

38 Branch
mozilla39
All
Windows NT
crash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 fixed, firefox39 fixed)

Details

(crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
This bug was filed from the Socorro interface and is 
report bp-da0c6d4c-da4d-472b-b2a7-b0c9d2150224.
=============================================================

New crash signature in v38.  Low volume.  All on Windows so far.

mozilla::plugins::PluginAsyncSurrogate::DestroyAsyncStream(_NPStream*)
mozilla::plugins::PluginAsyncSurrogate::NotifyAsyncInitFailed()
mozilla::plugins::PluginInstanceParent::RecvAsyncNPP_NewResult(short const&)
mozilla::plugins::PPluginInstanceParent::OnMessageReceived(IPC::Message const&)
mozilla::plugins::PPluginModuleParent::OnMessageReceived(IPC::Message const&)
mozilla::ipc::MessageChannel::OnMaybeDequeueOne()
RunnableMethod<mozilla::ipc::MessageChannel, void ( mozilla::ipc::MessageChannel::*)(void), Tuple0>::Run()
MessageLoop::DoWork()
mozilla::ipc::DoWorkRunnable::Run()
nsThread::ProcessNextEvent(bool, bool*)
mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)
mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)
MessageLoop::RunHandler()
MessageLoop::Run()
nsBaseAppShell::Run()
nsAppShell::Run()
XRE_RunAppShell
...


More Reports:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Aplugins%3A%3APluginAsyncSurrogate%3A%3ADestroyAsyncStream%28_NPStream%2A%29

Updated

3 years ago
Flags: needinfo?(aklotz)
On it.
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Flags: needinfo?(aklotz)
Blocks: 1116806
(Reporter)

Comment 2

3 years ago
Another signature that appears to be the same underlying problem:
https://crash-stats.mozilla.com/report/list?range_unit=days&range_value=28&signature=nsNPAPIPluginStreamListener%3A%3AResumeRequest()
Crash Signature: [@ mozilla::plugins::PluginAsyncSurrogate::DestroyAsyncStream(_NPStream*)] → [@ mozilla::plugins::PluginAsyncSurrogate::DestroyAsyncStream(_NPStream*)] [@ nsNPAPIPluginStreamListener::ResumeRequest() ]
Created attachment 8570664 [details] [diff] [review]
Hook PluginAsyncSurrogate into NPP_DestroyStream

We were trying to destroy NPStream instances that were already gone. Sending NPP_DestroyStream notifications to PluginAsyncSurrogate should take care of that.
Attachment #8570664 - Flags: review?(jmathies)

Comment 4

3 years ago
Comment on attachment 8570664 [details] [diff] [review]
Hook PluginAsyncSurrogate into NPP_DestroyStream

Review of attachment 8570664 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/plugins/ipc/PluginAsyncSurrogate.cpp
@@ +269,5 @@
>  
> +NPError
> +PluginAsyncSurrogate::NPP_DestroyStream(NPStream* aStream, NPReason aReason)
> +{
> +  for (PRUint32 i = 0, len = mPendingNewStreamCalls.Length(); i < len; ++i) {

nit - uint32_t, also personal pet peve - I prefer longer generic index names, like 'idx' or 'index' vs. 'i'.
Attachment #8570664 - Flags: review?(jmathies) → review+
Crash Signature: [@ mozilla::plugins::PluginAsyncSurrogate::DestroyAsyncStream(_NPStream*)] [@ nsNPAPIPluginStreamListener::ResumeRequest() ] → [@ mozilla::plugins::PluginAsyncSurrogate::DestroyAsyncStream(_NPStream*)] [@ nsNPAPIPluginStreamListener::ResumeRequest() ] [@ mozilla::plugins::PluginAsyncSurrogate::NotifyAsyncInitFailed() ]
Created attachment 8573614 [details] [diff] [review]
Hook PluginAsyncSurrogate into NPP_DestroyStream (r2)

Comments addresssed, carrying forward r+.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9cdc90bd4c32
Attachment #8570664 - Attachment is obsolete: true
Attachment #8573614 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/9cdc90bd4c32
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
I'm still seeing crash reports for this bug and I think I know why. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 8576789 [details] [diff] [review]
Part 2: Ensure NPP_DestroyStream can be called during async init

I converted a boolean that indicated stream state into a 3-state enum. Most use cases use the equivalent states that existed before (stopped or fully initialized). The one that changed is the call to NPP_DestroyStream: we should make that call even if we're not fully initialized yet.
Attachment #8576789 - Flags: review?(jmathies)

Comment 9

3 years ago
Comment on attachment 8576789 [details] [diff] [review]
Part 2: Ensure NPP_DestroyStream can be called during async init

Review of attachment 8576789 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/plugins/base/nsNPAPIPluginStreamListener.cpp
@@ +229,5 @@
>  
>    NPP npp;
>    mInst->GetNPP(&npp);
>  
> +  if (mStreamState >= eNewStreamCalled && pluginFunctions->destroystream) {

nice find!
Attachment #8576789 - Flags: review?(jmathies) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/00a32fed278e
https://hg.mozilla.org/mozilla-central/rev/00a32fed278e
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
Comment on attachment 8573614 [details] [diff] [review]
Hook PluginAsyncSurrogate into NPP_DestroyStream (r2)

Approval Request Comment
[Feature/regressing bug #]: Asynchronous Plugin Initialization
[User impact if declined]: Will not be able to enable this feature on 38 due to crashing
[Describe test coverage new/current, TreeHerder]: Running on nightly, associated crash is gone
[Risks and why]: Low, cause of crash is well understood, fix works
[String/UUID change made/needed]: None
Attachment #8573614 - Flags: approval-mozilla-aurora?
Comment on attachment 8576789 [details] [diff] [review]
Part 2: Ensure NPP_DestroyStream can be called during async init

Approval Request Comment
[Feature/regressing bug #]: Asynchronous Plugin Initialization
[User impact if declined]: Will not be able to enable this feature on 38 due to crashing
[Describe test coverage new/current, TreeHerder]: Running on nightly, associated crash is gone
[Risks and why]: Low, cause of crash is well understood, fix works
[String/UUID change made/needed]: None
Attachment #8576789 - Flags: approval-mozilla-aurora?
Attachment #8573614 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8576789 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox38: --- → affected
https://hg.mozilla.org/releases/mozilla-aurora/rev/9797a33e3b82
https://hg.mozilla.org/releases/mozilla-aurora/rev/bf384c919c33
status-firefox38: affected → fixed
Depends on: 1152890
There still seem to be plenty of instances of these crashes around, especially on Firefox 39 Aurora. Is this tracked separately somewhere?
You need to log in before you can comment on or make changes to this bug.