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

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Mats Palmgren (inactive)]

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

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

On it.

Comment 2

[Mats Palmgren (inactive)]

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()

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: 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.

Comment 4

[Jim Mathies [:jimm]]

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'.

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Hook PluginAsyncSurrogate into NPP_DestroyStream (r2)

Comments addresssed, carrying forward r+.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9cdc90bd4c32

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9cdc90bd4c32

Comment 7

[(No longer employed by Mozilla) Aaron Klotz]

I'm still seeing crash reports for this bug and I think I know why. Reopening.

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: 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.

Comment 9

[Jim Mathies [:jimm]]

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!

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/00a32fed278e

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/00a32fed278e

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9797a33e3b82
https://hg.mozilla.org/releases/mozilla-aurora/rev/bf384c919c33

Comment 15

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

There still seem to be plenty of instances of these crashes around, especially on Firefox 39 Aurora. Is this tracked separately somewhere?