Changeset 86495 in vbox

Timestamp:

Oct 8, 2020 1:04:26 PM (4 years ago)

Author:

vboxsync

Message:

VBoxSVC/HostPowerServiceLinux: Use RTThreadPoke on the notification thread if it doesn't seem to respond to dbus_connection_close() after 10 ms. Also, grab an extra reference on the connection so both the thread and the destruction have one to drop, otherwise we'd be in trouble if the other side terminates the connection and the thread exits early. bugref:9841

Location:

trunk

Files:

• include/VBox/dbus-calls.h (modified) (1 diff)
• src/VBox/Main/src-server/linux/HostPowerLinux.cpp (modified) (6 diffs)

trunk/include/VBox/dbus-calls.h

@@ -45 +45 @@
  RT_PROXY_STUB(dbus_free_string_array, void, (char **str_array), \
                  (str_array)) \
+ RT_PROXY_STUB(dbus_connection_ref, DBusConnection *, (DBusConnection *connection), \
+                 (connection)) \
  RT_PROXY_STUB(dbus_connection_unref, void, (DBusConnection *connection), \
                  (connection)) \

trunk/src/VBox/Main/src-server/linux/HostPowerLinux.cpp

@@ -45 +45 @@
 HostPowerServiceLinux::HostPowerServiceLinux(VirtualBox *aVirtualBox)
   : HostPowerService(aVirtualBox)
-  , mThread(NULL)
+  , mThread(NIL_RTTHREAD)
   , mpConnection(NULL)
 {
@@ -76 +76 @@
     if (checkDBusError(&error, &mpConnection))
         return;
+
+    /* Grab another reference so that both the destruct and thread each has one: */
+    DBusConnection *pForAssert = dbus_connection_ref(mpConnection);
+    Assert(pForAssert == mpConnection); RT_NOREF(pForAssert);
+
     /* Create the new worker thread. */
     rc = RTThreadCreate(&mThread, HostPowerServiceLinux::powerChangeNotificationThread, this, 0 /* cbStack */,
                         RTTHREADTYPE_MSG_PUMP, RTTHREADFLAGS_WAITABLE, "MainPower");
     if (RT_FAILURE(rc))
+    {
         LogRel(("HostPowerServiceLinux: RTThreadCreate failed with %Rrc\n", rc));
+        dbus_connection_unref(mpConnection);
+    }
 }
 
@@ -86 +94 @@
 HostPowerServiceLinux::~HostPowerServiceLinux()
 {
-    int rc;
-    RTMSINTERVAL cMillies = 5000;
-
     /* Closing the connection should cause the event loop to exit. */
     LogFunc((": Stopping thread\n"));
     if (mpConnection)
+    {
         dbus_connection_close(mpConnection);
+        dbus_connection_unref(mpConnection);
+        mpConnection = NULL;
+    }
 
-    rc = RTThreadWait(mThread, cMillies, NULL);
-    if (rc != VINF_SUCCESS)
-        LogRelThisFunc(("RTThreadWait() for %u ms failed with %Rrc\n", cMillies, rc));
-    mThread = NIL_RTTHREAD;
+    if (mThread != NIL_RTTHREAD)
+    {
+        /* HACK ALERT! This poke call should _not_ be necessary as dbus_connection_close()
+                       should close the socket and force the poll/dbus_connection_read_write
+                       call to return with POLLHUP/FALSE.  It does so when stepping it in the
+                       debugger, but not in real life (asan build; dbus-1.12.20-1.fc32; linux 5.8).
+
+                       Poking the thread is a crude crude way to wake it up from whatever
+                       stuff it's actually blocked on and realize that the connection has
+                       been dropped. */
+
+        uint64_t msElapsed = RTTimeMilliTS();
+        int vrc = RTThreadWait(mThread, 10 /*ms*/, NULL);
+        if (RT_FAILURE(vrc))
+        {
+            RTThreadPoke(mThread);
+            vrc = RTThreadWait(mThread, RT_MS_5SEC, NULL);
+        }
+        msElapsed = RTTimeMilliTS() - msElapsed;
+        if (vrc != VINF_SUCCESS)
+            LogRelThisFunc(("RTThreadWait() failed after %llu ms: %Rrc\n", msElapsed, vrc));
+        mThread = NIL_RTTHREAD;
+    }
 }
 
@@ -114 +142 @@
         for (;;)
         {
-            DBusMessageIter args;
-            dbus_bool_t fSuspend;
-
             pMessage = dbus_connection_pop_message(pConnection);
             if (pMessage == NULL)
                 break;
+
             /* The systemd-logind interface notification. */
+            DBusMessageIter args;
             if (   dbus_message_is_signal(pMessage, "org.freedesktop.login1.Manager", "PrepareForSleep")
                 && dbus_message_iter_init(pMessage, &args)
                 && dbus_message_iter_get_arg_type(&args) == DBUS_TYPE_BOOLEAN)
             {
+                dbus_bool_t fSuspend;
                 dbus_message_iter_get_basic(&args, &fSuspend);
+
                 /* Trinary operator does not work here as Reason_... is an
                  * anonymous enum. */
@@ -133 +162 @@
                     pPowerObj->notify(Reason_HostResume);
             }
+
             /* The UPowerd interface notifications.  Sleeping is the older one,
              * NotifySleep the newer.  This gives us one second grace before the
@@ -142 +172 @@
                 || dbus_message_is_signal(pMessage, "org.freedesktop.UPower", "NotifyResume"))
                 pPowerObj->notify(Reason_HostResume);
+
             /* Free local resources held for the message. */
             dbus_message_unref(pMessage);
         }
     }
+
     /* Close the socket or whatever underlying the connection. */
     dbus_connection_close(pConnection);
+
     /* Free in-process resources used for the now-closed connection. */
     dbus_connection_unref(pConnection);
+
     Log(("HostPowerServiceLinux: Exiting thread\n"));
     return VINF_SUCCESS;