Cleanup CheckPluginStopEvent, prevent possible leak of running plugin

RESOLVED FIXED in mozilla34

Status

()

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

People

(Reporter: johns, Assigned: johns)

Tracking

Trunk
mozilla34
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
While looking at the codepaths for bug 883134, I found yet-another-reentrance issue in this code-- we may clear mPendingCheckPluginStopEvent after calling StopPluginInstance in CheckPluginStopEvent::Run, but StopPluginInstance is re-entrant. This could lead to:

> StopPluginInstance->Script->Setup New Plugin->Script->DestroyContent->QueueCheckPluginStopEvent
(Assignee)

Comment 1

3 years ago
Created attachment 8459728 [details] [diff] [review]
Cleanup CheckPluginStopEvent, prevent possible leak of running plugin
Attachment #8459728 - Flags: review?(benjamin)
(Assignee)

Comment 2

3 years ago
(In reply to John Schoenick [:johns] from comment #1)
> Created attachment 8459728 [details] [diff] [review]
> Cleanup CheckPluginStopEvent, prevent possible leak of running plugin

Annnd the explanation I put in the commit message instead of the comment field by accident:

Bug 1041691 - Cleanup CheckPluginStopEvent, prevent possible leak of running plugin
So I don't think this is behind bug 883134, but it's more potential re-entrance insanity cleanup that can't hurt.

The actual bug fixed here is nulling mPendingCheckPluginStopEvent at the end of CheckPluginStopEvent::Run, after StopPluginInstance(), which may have re-entered, thus potentially blocking a valid plugin stop event which could be the last-chance stop of a plugin when content is destroyed.

This also stops calling UnloadObject() inside CheckPluginStopEvent, which just throws away extra state for no apparent reason, avoids queuing some unnecessary events, and cleans up the code a little.
(Assignee)

Comment 3

3 years ago
https://tbpl.mozilla.org/?tree=Try&rev=1e3582848cfa

Updated

3 years ago
Attachment #8459728 - Flags: review?(benjamin) → review+
(Assignee)

Comment 4

3 years ago
Comment on attachment 8459728 [details] [diff] [review]
Cleanup CheckPluginStopEvent, prevent possible leak of running plugin

https://hg.mozilla.org/integration/mozilla-inbound/rev/445f556f7a98
Attachment #8459728 - Flags: checkin+

Comment 5

3 years ago
https://hg.mozilla.org/mozilla-central/rev/445f556f7a98
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.