Closed Bug 1170343 Opened 7 years ago Closed 7 years ago

_releasevariantvalue does not have ENSURE_PLUGIN_THREAD

Categories

(Core :: Plug-ins, defect)

defect
Not set
normal

Tracking

()

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.
You need to log in before you can comment on or make changes to this bug.