Bug 544945, part 1: Detect nested glib event loops in the plugin subprocess. r=karlt
authorChris Jones <jones.chris.g@gmail.com>
Tue, 16 Feb 2010 12:44:24 -0600
changeset 38684 569dede83071a1f7eb9ce79a1537f6e822ee0f88
parent 38683 6df50088fa186f7bd702f1ad6ad2a7cd0494d84c
child 38685 d553de3fad407432815d87555ec5b854454b3657
push id11800
push userbsmedberg@mozilla.com
push dateThu, 25 Feb 2010 06:51:28 +0000
treeherderautoland@9a4b73f92f0e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskarlt
bugs544945
milestone1.9.3a2pre
Bug 544945, part 1: Detect nested glib event loops in the plugin subprocess. r=karlt
dom/plugins/PluginModuleChild.cpp
dom/plugins/PluginModuleChild.h
--- a/dom/plugins/PluginModuleChild.cpp
+++ b/dom/plugins/PluginModuleChild.cpp
@@ -76,18 +76,19 @@ static QApplication *gQApp = nsnull;
 
 
 PluginModuleChild::PluginModuleChild() :
     mLibrary(0),
     mInitializeFunc(0),
     mShutdownFunc(0)
 #ifdef OS_WIN
   , mGetEntryPointsFunc(0)
+#elif defined(MOZ_WIDGET_GTK2)
+  , mNestedLoopTimerId(0)
 #endif
-//  ,mNextInstanceId(0)
 {
     NS_ASSERTION(!gInstance, "Something terribly wrong here!");
     memset(&mFunctions, 0, sizeof(mFunctions));
     memset(&mSavedData, 0, sizeof(mSavedData));
     gInstance = this;
 }
 
 PluginModuleChild::~PluginModuleChild()
@@ -236,22 +237,98 @@ wrap_gtk_plug_embedded(GtkPlug* plug) {
         // https://bugzilla.gnome.org/show_bug.cgi?id=607061
         g_object_ref(socket_window);
     }
 
     if (*real_gtk_plug_embedded) {
         (*real_gtk_plug_embedded)(plug);
     }
 }
+
+//
+// The next four constants are knobs that can be tuned.  They trade
+// off potential UI lag from delayed event processing with CPU time.
+//
+static const gint kNestedLoopDetectorPriority = G_PRIORITY_HIGH_IDLE;
+// 90ms so that we can hopefully break livelocks before the user
+// notices UI lag (100ms)
+static const guint kNestedLoopDetectorIntervalMs = 90;
+
+static const gint kBrowserEventPriority = G_PRIORITY_HIGH_IDLE;
+static const guint kBrowserEventIntervalMs = 10;
+
+// static
+gboolean
+PluginModuleChild::DetectNestedEventLoop(gpointer data)
+{
+    PluginModuleChild* pmc = static_cast<PluginModuleChild*>(data);
+
+    NS_ABORT_IF_FALSE(0 != pmc->mNestedLoopTimerId,
+                      "callback after descheduling");
+    NS_ABORT_IF_FALSE(1 < g_main_depth(),
+                      "not canceled before returning to main event loop!");
+
+    PLUGIN_LOG_DEBUG(("Detected nested glib event loop"));
+
+    // just detected a nested loop; start a timer that will
+    // periodically rpc-call back into the browser and process some
+    // events
+    pmc->mNestedLoopTimerId =
+        g_timeout_add_full(kBrowserEventPriority,
+                           kBrowserEventIntervalMs,
+                           PluginModuleChild::ProcessBrowserEvents,
+                           data,
+                           NULL);
+    // cancel the nested-loop detection timer
+    return FALSE;
+}
+
+// static
+gboolean
+PluginModuleChild::ProcessBrowserEvents(gpointer data)
+{
+    NS_ABORT_IF_FALSE(1 < g_main_depth(),
+                      "not canceled before returning to main event loop!");
+
+    PluginModuleChild* pmc = static_cast<PluginModuleChild*>(data);
+
+    PLUGIN_LOG_DEBUG(("FIXME/bug 544945: rpc-call to browser to process a few events"));
+
+    return TRUE;
+}
+
+void
+PluginModuleChild::EnteredCxxStack()
+{
+    NS_ABORT_IF_FALSE(0 == mNestedLoopTimerId,
+                      "previous timer not descheduled");
+
+    mNestedLoopTimerId =
+        g_timeout_add_full(kNestedLoopDetectorPriority,
+                           kNestedLoopDetectorIntervalMs,
+                           PluginModuleChild::DetectNestedEventLoop,
+                           this,
+                           NULL);
+}
+
+void
+PluginModuleChild::ExitedCxxStack()
+{
+    NS_ABORT_IF_FALSE(0 < mNestedLoopTimerId,
+                      "nested loop timeout not scheduled");
+
+    g_source_remove(mNestedLoopTimerId);
+    mNestedLoopTimerId = 0;
+}
+
 #endif
 
 bool
 PluginModuleChild::InitGraphics()
 {
-    // FIXME/cjones: is this the place for this?
 #if defined(MOZ_WIDGET_GTK2)
     // Work around plugins that don't interact well with GDK
     // client-side windows.
     PR_SetEnv("GDK_NATIVE_WINDOWS=1");
 
     gtk_init(0, 0);
 
     // GtkPlug is a static class so will leak anyway but this ref makes sure.
--- a/dom/plugins/PluginModuleChild.h
+++ b/dom/plugins/PluginModuleChild.h
@@ -157,32 +157,75 @@ public:
     static NPObject* NP_CALLBACK NPN_RetainObject(NPObject* aNPObj);
     /**
      * The child implementation of NPN_ReleaseObject.
      */
     static void NP_CALLBACK NPN_ReleaseObject(NPObject* aNPObj);
 
 private:
     bool InitGraphics();
+#if defined(MOZ_WIDGET_GTK2)
+    static gboolean DetectNestedEventLoop(gpointer data);
+    static gboolean ProcessBrowserEvents(gpointer data);
+
+    NS_OVERRIDE
+    virtual void EnteredCxxStack();
+    NS_OVERRIDE
+    virtual void ExitedCxxStack();
+#endif
 
     std::string mPluginFilename;
     PRLibrary* mLibrary;
     nsCString mUserAgent;
 
     // we get this from the plugin
 #ifdef OS_POSIX
     NP_PLUGINUNIXINIT mInitializeFunc;
 #elif OS_WIN
     NP_PLUGININIT mInitializeFunc;
     NP_GETENTRYPOINTS mGetEntryPointsFunc;
 #endif
+
     NP_PLUGINSHUTDOWN mShutdownFunc;
     NPPluginFuncs mFunctions;
     NPSavedData mSavedData;
 
+#if defined(MOZ_WIDGET_GTK2)
+    // If a plugin spins a nested glib event loop in response to a
+    // synchronous IPC message from the browser, the loop might break
+    // only after the browser responds to a request sent by the
+    // plugin.  This can happen if a plugin uses gtk's synchronous
+    // copy/paste, for example.  But because the browser is blocked on
+    // a condvar, it can't respond to the request.  This situation
+    // isn't technically a deadlock, but the symptoms are basically
+    // the same from the user's perspective.
+    //
+    // We take two steps to prevent this
+    //
+    //  (1) Detect nested event loops spun by the plugin.  This is
+    //      done by scheduling a glib timer event in the plugin
+    //      process whenever the browser might block on the plugin.
+    //      If the plugin indeed spins a nested loop, this timer event
+    //      will fire "soon" thereafter.
+    //
+    //  (2) When a nested loop is detected, deschedule the
+    //      nested-loop-detection timer and in its place, schedule
+    //      another timer that periodically calls back into the
+    //      browser and spins a mini event loop.  This mini event loop
+    //      processes a handful of pending native events.
+    //
+    // Because only timer (1) or (2) (or neither) may be active at any
+    // point in time, we use the same member variable
+    // |mNestedLoopTimerId| to refer to both.
+    //
+    // When the browser no longer might be blocked on a plugin's IPC
+    // response, we deschedule whichever of (1) or (2) is active.
+    guint mNestedLoopTimerId;
+#endif
+
     struct NPObjectData : public nsPtrHashKey<NPObject>
     {
         NPObjectData(const NPObject* key)
             : nsPtrHashKey<NPObject>(key)
             , instance(NULL)
             , actor(NULL)
         { }