493439 - Quake Live plugin disables crash reporter

Status: RESOLVED INCOMPLETE

(Plugins Graveyard :: Quake Live, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

This is basically similar to bug 422308, which was a bug in older Flash 9.  To reproduce:

1) Go to http://www.quakelive.com/, set up an account, install plugin
2) Enter game (tutorial game works fine)
3) Select Crash Me Now from Ted's crashme extension

-> Windows crash dialog, no breakpad.

Battlefield Heroes (another similar in-browser game plugin) might be doing the same thing.  It's not clear whether QL is resetting the exception handler state on every plugin entry, or only on initialization.

Comment 1

[Johnny Stenback (:jst)]

Is there anything *we* can do about this?

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Well, Flash was calling SetUnhandledExceptionFilter apparently in every one of its function impls; we probably don't want to wrap every call into a plugin with saving/resetting this, but at the very least we could wrap any initialization/non-perf-critical functions.

This would also give us a spot to check/reset the FPU mode, to try to avoid breaking JavaScript if a plugin initializes DirectX (which dumps the FPU into single-precision mode).

Actually, depending on how fast those functions are, we could even do it on every plugin function entry/exit as well.  I'd assume that SetUnhandledExceptionFilter just sets a global (it's not even thread local, as far as I can tell), and the fpu mode is available in a fpu status reg, so both of those should be fast operations.

Comment 3

[Johnny Stenback (:jst)]

I don't know that we'd want to go there, I'd much rather see the plugins behave and not leave things in a different state than what it was when they changed the state. I certainly wouldn't mind notifying about this one way or another, but I'm not sure what good ways we have to do that which would actually cause plugin developers to get the message. Either way, I really don't think I'd hold the release for this. blocking-

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

We can't control plugins though, and until we can get them out of process, this leads to a pretty crappy user experience.  Wrapping some calls into the plugin with a simple object that'll save/restore some state should have a negligible effect on perf, and will help us collect crash data that we can then go to the plugin vendor with..

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I think we *do* want to go there -- we should restore our invariants as aggressively as we can without slaughtering perf.

Comment 6

[Johnny Stenback (:jst)]

I disagree. There's several cases where we can not do anything about this (short of sticking fixup and restore code in the guts of our event loop), like if a plugin messes with any of the states in question (and who knows what other state) and goes back to our event loop, or if they create child widgets and do this as a response to events there, we're in the same boat again. I'd much rather see plugins not stomp over stuff that affects others and leave things as such than us trying to fix up after them, and fully knowing that we can only do that in some cases at best.

Comment 7

[Johnny Stenback (:jst)]

Now having said that, restoring the crash reporter if plugin initialization wipes it out I think would be ok (assuming that actually is enough to fix this bug), but I don't like the idea of attempting to do that across the board for the whole plugin API.

Comment 8

[Benjamin Smedberg]

Closing old bugs in the Plugins component. We aren't going to track issues in 3rd-party plugins in the Mozilla bug tracker. In addition, support for NPAPI plugins will be removed at the end of this year; for more details see the post at https://blog.mozilla.org/futurereleases/2015/10/08/npapi-plugins-in-firefox/

If there is a serious bug in Firefox, it needs to be filed in the "Core" product, "Plug-Ins" component.