Closed Bug 1170343 Opened 7 years ago Closed 7 years ago

_releasevariantvalue does not have ENSURE_PLUGIN_THREAD

Categories

(Core Graveyard :: Plug-ins, defect)

defect
Not set
normal

Tracking

(firefox41 fixed)

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: mccr8, Assigned: benjamin)

References

Details

Attachments

(1 file)

Bug 521118 added thread safety checks to plugin IPC entry points (that return on failure), but a few methods, including _releasevariantvalue, were left as NS_ASSERTIONs, "for consistency with in-process plugins".

Anywho, it turns out that Java calls NPN_ReleaseObject off the main thread, which then tries to send IPC, which hits the new IPDL thread safety assertions added in bug 1155375:

https://crash-stats.mozilla.com/report/index/afec212c-49ac-474e-89ee-2783c2150531

So maybe we want to reconsider adding in ENSURE_PLUGIN_THREAD to more plugin entry points.
Flags: needinfo?(aklotz)
Oh good grief. I guess I should stop being surprised at Java's idiocy.

If we add a non-crashing check to this, we're guaranteeing leaks. Do we know how common this is? It may be that crashing is the better choice here.
Flags: needinfo?(aklotz)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> If we add a non-crashing check to this, we're guaranteeing leaks. Do we know
> how common this is? It may be that crashing is the better choice here.

I think I only saw one stack like this, but these assertions have only gotten as far as 40.
Let's make it more fatal and see what happens. I cannot think of a way to make it safe otherwise.
Going to open this up and try making AssertPluginThread a release-mode assertion.
Assignee: nobody → benjamin
Group: core-security
Status: NEW → ASSIGNED
Bug 1170343 - Use release-mode asserts when plugins making NPAPI calls on the wrong thread, r?mccr8
Attachment #8617958 - Flags: review?(continuation)
Comment on attachment 8617958 [details]
MozReview Request: Bug 1170343 - Use release-mode asserts when plugins making NPAPI calls on the wrong thread, r?mccr8

Thanks.
Attachment #8617958 - Flags: review?(continuation) → review+
https://hg.mozilla.org/mozilla-central/rev/6f7f683a0ccc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Depends on: 1175147
Filed comment 9 as bug 1175147
This was still showing up in my reviewboard queue, I guess because I was using it wrong, so I just did that to get rid of it.
Well, it is still in my review board queue.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.