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

RESOLVED FIXED in Firefox 38

Status

()

--
critical
RESOLVED FIXED
4 years ago
4 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

4 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

4 years ago
Flags: needinfo?(aklotz)
(Assignee)

Comment 1

4 years ago
On it.
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Flags: needinfo?(aklotz)
(Assignee)

Updated

4 years ago
Blocks: 1116806
(Reporter)

Comment 2

4 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() ]
(Assignee)

Comment 3

4 years ago
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 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+
(Assignee)

Updated

4 years ago
Crash Signature: [@ mozilla::plugins::PluginAsyncSurrogate::DestroyAsyncStream(_NPStream*)] [@ nsNPAPIPluginStreamListener::ResumeRequest() ] → [@ mozilla::plugins::PluginAsyncSurrogate::DestroyAsyncStream(_NPStream*)] [@ nsNPAPIPluginStreamListener::ResumeRequest() ] [@ mozilla::plugins::PluginAsyncSurrogate::NotifyAsyncInitFailed() ]
(Assignee)

Comment 5

4 years ago
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: 4 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
(Assignee)

Comment 7

4 years ago
I'm still seeing crash reports for this bug and I think I know why. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 8

4 years ago
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 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
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 12

4 years ago
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?
(Assignee)

Comment 13

4 years ago
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
(Assignee)

Updated

4 years ago
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.