Changeset 90057 in vbox for trunk/src/VBox/Additions

Timestamp:

Jul 6, 2021 11:19:50 AM (4 years ago)

Author:

vboxsync

Message:

Additions: X11: seamless: prevent hang on shutdown (2), bugref:10032.

When X11 is no longer running, XCloseDisplay() call will indefinitely
hang on internal X11 mutex preventing service from shutdown. In this
commit, on shutdown code path, we examine if XOpenDisplay() returns non-NULL,
meaning that X server is still running, and only after this XCloseDisplay()
is called. In theory, this check still might not work (in case of bad timing),
but in general it improves sutuation and service is able to terminate smoothly.

In addition, before calling XNextEvent(), X11 monitor thread attempts to examine
with XPending() if there are some events actually pending. This is needed because
XNextEvent() might wait forever for the next event to arrive and block service
termination.

Location:

trunk/src/VBox/Additions/x11/VBoxClient

Files:

• seamless-x11.cpp (modified) (4 diffs)
• testcase/tstSeamlessX11-auto.cpp (modified) (1 diff)

trunk/src/VBox/Additions/x11/VBoxClient/seamless-x11.cpp

@@ -24 +24 @@
 #include <iprt/assert.h>
 #include <iprt/vector.h>
+#include <iprt/thread.h>
 #include <VBox/log.h>
 
@@ -105 +106 @@
         stop();
     mHostCallback = NULL;
-    if (mDisplay)
-        XCloseDisplay(mDisplay);
-    mDisplay = NULL;
+
+    /* Before closing a Display, make sure X11 is still running. The indicator
+     * that is when XOpenDisplay() returns non NULL. If it is not a
+     * case, XCloseDisplay() will hang on internal X11 mutex forever. */
+    Display *pDisplay = XOpenDisplay(NULL);
+    if (pDisplay)
+    {
+        XCloseDisplay(pDisplay);
+        if (mDisplay)
+        {
+            XCloseDisplay(mDisplay);
+            mDisplay = NULL;
+        }
+    }
+
     if (mpRects)
     {
@@ -331 +344 @@
     }
     mChanged = false;
-    /* We execute this even when seamless is disabled, as it also waits for
-     * enable and disable notification. */
-    XNextEvent(mDisplay, &event);
+
+    if (XPending(mDisplay) > 0)
+    {
+        /* We execute this even when seamless is disabled, as it also waits for
+         * enable and disable notification. */
+        XNextEvent(mDisplay, &event);
+    } else
+    {
+        /* This function is called in a loop by upper layer. In order to
+         * prevent CPU spinning, sleep a bit before returning. */
+        RTThreadSleep(300 /* ms */);
+        return;
+    }
+
     if (!mEnabled)
         return;
@@ -524 +548 @@
     if (pDisplay == NULL)
     {
-        VBClLogFatalError("Failed to open X11 display\n");
+        VBClLogError("Failed to open X11 display\n");
         return false;
     }

trunk/src/VBox/Additions/x11/VBoxClient/testcase/tstSeamlessX11-auto.cpp

@@ -316 +316 @@
     event_return->xmap.window = g_SmlsEventWindow;
     return True;
+}
+
+/* Mock XNextEvent(): this also should not be needed. Just in case, always
+ * return that at least one event is pending to be processed. */
+extern "C" int XPending(Display *display);
+int XPending(Display *display)
+{
+    RT_NOREF1(display);
+    return 1;
 }