[Bug 724587] Re: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
Launchpad has imported 7 comments from the remote bug at https://bugzilla.redhat.com/show_bug.cgi?id=680279. If you reply to an imported comment from within Launchpad, your comment will be sent to the remote bug automatically. Read more about Launchpad's inter-bugtracker facilities at https://help.launchpad.net/InterBugTracking. On 2011-02-24T22:00:08+00:00 David wrote: This is the same report as https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587 on Ubuntu; I'm filing here too because, as far as I can tell, nspluginwrapper no longer has an upstream. --- nspluginwrapper has a race condition during NPP_Destroy (called when a tab is closed) that can crash the plugin. The race happens particularly often when another tab has a video playing; I suspect this is because it causes enough traffic over the IPC to delay the processes and trigger the race. If NPP_Destroy is called by the wrapper process at the same time the viewer (plugin) process makes some call, then, from the plugin's perspective, its call to NPN_InvalidateRect resulted in the plugin instance being destroyed from under its feet. This is, of course, nonsense, so Flash shortly crashes to let us know how silly we are being. :-) I've written patches for this issue here https://github.com/davidben/nspluginwrapper/commits/master Only the second of the two patches is strictly relevant. (The other is a separate race I came across in a previous iteration of this patch.) It detects when NPP_Destroy is being called at an unsafe point and delays it to another message loop iteration. With the caveat that requests can't be reordered. So, when it must, the patch lies to the wrapper about NPP_Destroy's return values. Any delayed NPSavedData gets discarded. That said, I've never seen Flash use this feature, and the docs do allow the browser to discard them arbitrarily. The relevant bug in Chromium is here: http://code.google.com/p/chromium/issues/detail?id=53940 Reply at: https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587/comments/3 On 2011-03-08T11:01:32+00:00 Peter wrote: Created attachment 482872 Delay calls to NPP_Destroy when the plugin instance is on the stack. Delay calls to NPP_Destroy when the plugin instance is on the stack Otherwise, from the plugin's perspective, NPN_InvalidateRect results in the plugin instance exploding. One can hardly blame Adobe that Flash crashes in this situation. Unfortunately, because we attempt to speak synchronous NPAPI on both ends and there's the additional SYNC mechanism forcing a particular order, we cannot reorder the calls. As a result, when NPP_Destroy must be delayed, we lie to the browser and actually destroy the plugin later. This means, however, that any NPSavedData provided by the plugin is ignored. In this case, a warning is emitted. We do a slightly more conservative check and delay whenever we have any invoke on the call stack, be it this instance or any other. This is better than checking the refcount because any NPObjectInfo will hold a reference. Reply at: https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587/comments/4 On 2011-03-08T11:02:56+00:00 Peter wrote: Created attachment 482873 Make delayed_calls_process re-entrant And I also attach all the other relevant patches. Reply at: https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587/comments/5 On 2011-03-08T11:04:03+00:00 Peter wrote: Created attachment 482874 Delay calls to NPP_Destroy when the plugin instance is on the stack Reply at: https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587/comments/6 On 2011-03-08T11:05:09+00:00 Peter wrote: Created attachment 482875 Delay calls to NPP_Destroy when the plugin instance is on the stack. Reply at: https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587/comments/7 On 2011-03-09T10:17:46+00:00 Fedora wrote: nspluginwrapper-1.3.0-18.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/nspluginwrapper-1.3.0-18.fc15 Reply at: https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587/comments/8 On 2011-03-26T05:08:37+00:00 Fedora wrote: nspluginwrapper-1.3.0-18.fc15 has been pushed to the Fedora 15 stable repository. If problems still persist, please make note of it in this bug report. Reply at: https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587/comments/9 ** Changed in: nspluginwrapper (Fedora) Status:
[Bug 724587] Re: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
** Branch linked: lp:ubuntu/nspluginwrapper -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/724587 Title: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 724587] Re: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
This bug was fixed in the package nspluginwrapper - 1.2.2-0ubuntu9 --- nspluginwrapper (1.2.2-0ubuntu9) natty; urgency=low [ David Benjamin ] * Fix a re-entrancy bug with delayed_calls_process and a race condition (causing a crash) when a plugin instance is destroyed. (LP: #724587) - add debian/patches/008_delayed_calls_process_reentrant.diff - add debian/patches/009_npp_destroy_crash.diff - update debian/patches/series [ Anders Kaseorg ] * Fix FTBFS due to configure test broken by libxt multiarch transition. - add debian/patches/010_fix_Xt_test.diff - update debian/patches/series * Update Maintainer to Ubuntu Developers . -- Anders KaseorgTue, 29 Mar 2011 14:33:16 -0400 ** Changed in: nspluginwrapper (Ubuntu) Status: New => Fix Released -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/724587 Title: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 724587] Re: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
Thanks, David and Anders! I pushed this to Ubuntu. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/724587 Title: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 724587] Re: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
Here’s a debdiff including those patches. It also fixes a FTBFS due to a configure test broken by the libxt multiarch transition. ** Patch added: "nspluginwrapper_1.2.2-0ubuntu8_lp724587.debdiff" https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587/+attachment/1954187/+files/nspluginwrapper_1.2.2-0ubuntu8_lp724587.debdiff -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/724587 Title: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 724587] Re: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
** Also affects: nspluginwrapper (Fedora) via https://bugzilla.redhat.com/show_bug.cgi?id=680279 Importance: Unknown Status: Unknown -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/724587 Title: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 724587] Re: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
https://bugzilla.redhat.com/show_bug.cgi?id=680279 The patched has made it into Fedora. ** Bug watch added: Red Hat Bugzilla #680279 https://bugzilla.redhat.com/show_bug.cgi?id=680279 -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/724587 Title: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 724587] Re: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
** Tags added: patch -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/724587 Title: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 724587] Re: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
And this is the second. ** Patch added: "[PATCH 2/2] Delay calls to NPP_Destroy when the plugin instance is on the stack" https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587/+attachment/1869594/+files/0002-Delay-calls-to-NPP_Destroy-when-the-plugin-instance-.patch -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/724587 Title: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 724587] Re: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
This is the first of two patches in the linked git repo. (The formatting might look off because nspluginwrapper assumes tabs are 4 spaces. It's kind of obnoxious.) ** Patch added: "[PATCH 1/2] Make delayed_calls_process re-entrant" https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587/+attachment/1869593/+files/0001-Make-delayed_calls_process-re-entrant.patch -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/724587 Title: [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
