Closed Bug 1136930 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Plug-ins, defect, critical)

38 Branch
All
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: mats, Assigned: aklotz)

References

Details

(Keywords: crash)

Crash Data

Attachments

(2 files, 1 obsolete file)

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
Flags: needinfo?(aklotz)
On it.
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Flags: needinfo?(aklotz)
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() ]
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 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() ]
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
Closed: 5 years ago
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 → ---
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 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/mozilla-central/rev/00a32fed278e
Status: REOPENED → RESOLVED
Closed: 5 years ago5 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+
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.