Closed Bug 1136930 Opened 8 years ago Closed 8 years ago

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

Categories

(Core Graveyard :: Plug-ins, defect)

38 Branch
All
Windows NT
defect
Not set
critical

Tracking

(firefox38 fixed, firefox39 fixed)

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

People

(Reporter: MatsPalmgren_bugz, Assigned: bugzilla)

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() ]
https://hg.mozilla.org/mozilla-central/rev/9cdc90bd4c32
Status: ASSIGNED → RESOLVED
Closed: 8 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: 8 years ago8 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?
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.