We can't do much about plugins sanitization taking huge time in a short time. So, alternatively, we can try to avoid waiting too much for it, in the end we don't get immediate benefits from knowing a plugin causes us to crash on shutdown, since we can't control their code. We can either set a timeout after which we give up waiting for the plugin, or we could just not wait at all. In any case we should try to retain the plugin sanitization telemetry.

David, thoughts?

Mostly, do we have any advantage in a soft timeout vs not waiting at all?

Well, soft timeout means "ok, let's wait at least 10 seconds", while not waiting at all means, if other sanitizations are fast "don't wait at all". I'd go for the soft timeout.

I'll try and get this done now-ish, if we agree.

Sure, the only thing I care about is that FX_SANITIZE_PLUGINS keeps accounting for the whole time, not the timeout.

above the "workaround" please add a TODO (bug 1249333) so if we get any final solution for that we can remove the soft timeout.

Would the soft-timeout only affect plugin sanitization during shutdown or also when selecting the appropriate menu entry while Firefox keeps running.

whimboo: It would just be less likely to crash during shutdown.

I'm asking because of bug 1241986, which shows us intermittent failures when sanitizing cookies. And as it looks like it exists because of the Flash plugin.

While investigating shutdown hangs/crashes, we determined that one of the sources was plugin cookie sanitization, which is sometimes insanely long. As a workaround, and until we have solved this in bug 1249333, this patch inroduces soft timeouts that will not hang/crash Firefox until sanitization of plugin cookies is complete. Review commit: https://reviewboard.mozilla.org/r/37971/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/37971/

(In reply to Henrik Skupin (:whimboo) from comment #9) > I'm asking because of bug 1241986, which shows us intermittent failures when > sanitizing cookies. And as it looks like it exists because of the Flash > plugin. It won't change a thing, we are just trying to not abort on purpose, showing the crash dialog to the user, if the problem is a plugin.

Comment on attachment 8726396 [details] MozReview Request: Bug 1253204 - Don't abort shutdown on plugins sanitization;r?mak https://reviewboard.mozilla.org/r/37971/#review34649 I'm sorry, imo this is too complex and abstract, it makes the cookies sanitization hard to read with only advantages for the plugins section. Also considered this should be uplifted, I'm not comfortable with the change. May we make something that only touches the plugins section of the cookies sanitizer? it shouldn't be hard, you can factor out only the plugins part.

Review commit: https://reviewboard.mozilla.org/r/38217/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/38217/

Tracking this for 46+ since we had problems with these types of shutdown crashes on 45 and this is part of the cleanup effort.

Comment on attachment 8726737 [details] MozReview Request: Bug 1253204 - Don't abort shutdown on plugins sanitization;r?mak https://reviewboard.mozilla.org/r/38217/#review35025

Comment on attachment 8726737 [details] MozReview Request: Bug 1253204 - Don't abort shutdown on plugins sanitization;r?mak Approval Request Comment [Feature/regressing bug #]: no bug [User impact if declined]: Crash on shutdown if a plugin takes many seconds to clear its data [Describe test coverage new/current, TreeHerder]: nightly [Risks and why]: the change is as self contained as possible, so the risk is limited to plugins clearing. We can track progress through crash-stats [String/UUID change made/needed]: none

Comment on attachment 8726737 [details] MozReview Request: Bug 1253204 - Don't abort shutdown on plugins sanitization;r?mak This will help improve shutdown hang situation, taking it.

Hi Kairo, Philipp, this is just an fyi in case we notice things getting worse instead of improving once this lands in 47 and 46.

Thanks, I have been following this bug already. ;-)

https://hg.mozilla.org/releases/mozilla-aurora/rev/45c68ee48929 https://hg.mozilla.org/releases/mozilla-beta/rev/654c0f3c051f

(In reply to Ritu Kothari (:ritu) from comment #20) > Hi Kairo, Philipp, this is just an fyi in case we notice things getting > worse instead of improving once this lands in 47 and 46. nothing obviously bad happened so far :))

(In reply to philipp from comment #23) > (In reply to Ritu Kothari (:ritu) from comment #20) > > Hi Kairo, Philipp, this is just an fyi in case we notice things getting > > worse instead of improving once this lands in 47 and 46. > > nothing obviously bad happened so far :)) That's great! Thank you for the follow up.

[bugday-20160323] Status: RESOLVED,FIXED -> UNVERIFIED Comments: STR: Not clear. Developer specific testing Component: Name Firefox Version 46.0b9 Build ID 20160322075646 Update Channel beta User Agent Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 OS Windows 7 SP1 x86_64 Expected Results: Developer specific testing Actual Results: As expected