Changeset 50346 in vbox

Timestamp:

Feb 6, 2014 2:40:20 PM (11 years ago)

Author:

vboxsync

Message:

Additions/x11/VBoxClient: more clean-up.

Location:

trunk/src/VBox/Additions/x11/VBoxClient

Files:

• seamless-x11.cpp (modified) (4 diffs)
• seamless-x11.h (modified) (4 diffs)
• seamless.cpp (modified) (14 diffs)
• seamless.h (modified) (7 diffs)
• testcase/tstSeamlessX11-auto.cpp (modified) (3 diffs)
• testcase/tstSeamlessX11.cpp (modified) (5 diffs)

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

@@ -96 +96 @@
  *
  * @note This class does not contain its own event thread, so an external thread must
- *       call nextEvent() for as long as events are wished.
+ *       call nextConfigurationEvent() for as long as events are wished.
  * @todo This function should switch the guest to fullscreen mode.
  */
@@ -289 +289 @@
  * @note Called from the guest event thread.
  */
-void SeamlessX11::nextEvent(void)
+void SeamlessX11::nextConfigurationEvent(void)
 {
     XEvent event;
@@ -299 +299 @@
     {
         updateRects();
-        mHost->notify(mpRects, mcRects);
+        mHost->sendRegionUpdate(mpRects, mcRects);
     }
     mChanged = false;
@@ -528 +528 @@
  * @note This function should only be called from the host event thread.
  */
-bool SeamlessX11::interruptEvent(void)
+bool SeamlessX11::interruptEventWait(void)
 {
     bool rc = false;

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

@@ -41 +41 @@
 {
 public:
-    virtual void notify(RTRECT *pRects, size_t cRects) = 0;
+    virtual void sendRegionUpdate(RTRECT *pRects, size_t cRects) = 0;
 };
 
@@ -216 +216 @@
     void uninit(void)
     {
-        if (0 != mHost)
-        {
+        if (mHost)
             stop();
-        }
-        mHost = 0;
+        mHost = NULL;
+        if (mDisplay)
+            XCloseDisplay(mDisplay);
+        mDisplay = NULL;
     }
 
@@ -237 +238 @@
 
     /** Process next event in the guest event queue - called by the event thread. */
-    void nextEvent(void);
+    void nextConfigurationEvent(void);
     /** Wake up the event thread if it is waiting for an event so that it can exit. */
-    bool interruptEvent(void);
+    bool interruptEventWait(void);
 
     /* Methods to handle X11 events.  These are public so that the unit test
@@ -255 +256 @@
     {
         uninit();
-        if (mDisplay)
-            XCloseDisplay(mDisplay);
     }
 };

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

@@ -31 +31 @@
 #include "seamless.h"
 
-/**
- * Start the service.
- * @returns iprt status value
+SeamlessMain::SeamlessMain(void)
+{
+    LogRelFlowFunc(("\n"));
+    mX11MonitorThread = NIL_RTTHREAD;
+    mX11MonitorThreadStopping = false;
+    mHostEventThread = NIL_RTTHREAD;
+    mHostEventThreadRunning = false;
+    mHostEventThreadStopping = false;
+}
+
+SeamlessMain::~SeamlessMain()
+{
+    LogRelFlowFunc(("\n"));
+    stop();
+}
+
+/**
+ * Start the main service thread which listens for host state change
+ * notifications.
+ * @returns iprt status value.  Service will be set to the stopped state on
+ *          failure.
  */
 int SeamlessMain::start(void)
 {
-    int rc = VERR_NOT_SUPPORTED;
-
-    LogRelFlowFunc(("\n"));
-    if (mThread)  /* Assertion */
-    {
-        LogRel(("VBoxClient: seamless service started twice!\n"));
-        return VERR_INTERNAL_ERROR;
-    }
-    rc = VbglR3CtlFilterMask(VMMDEV_EVENT_SEAMLESS_MODE_CHANGE_REQUEST, 0);
-    if (RT_FAILURE(rc))
-    {
-        LogRel(("VBoxClient (seamless): failed to set the guest IRQ filter mask, rc=%Rrc\n", rc));
-    }
-    rc = VbglR3SeamlessSetCap(true);
-    if (RT_SUCCESS(rc))
-    {
-        LogRel(("VBoxClient: enabled seamless capability on host.\n"));
+    int rc;
+    const char *pszStage;
+
+    LogRelFlowFunc(("\n"));
+    do {
+        pszStage = "Checking that we are not already running";
+        rc = VERR_INTERNAL_ERROR;
+        if (mHostEventThread)  /* Assertion */
+            break;
+        pszStage = "Connecting to the X server";
+        rc = mX11Monitor.init(this);
+        if (RT_FAILURE(rc))
+            break;
         /* Create a thread to wait for requests from the host.  This is currently
          * done on a separate thread as the main thread monitors the X11 server
@@ -59 +73 @@
         /** @todo Move the disconnection monitoring to its own thread (better, the
          *  VT monitor thread) and run this logic on the main service thread. */
-        rc = RTThreadCreate(&mThread, threadFunction, this, 0,
-                            RTTHREADTYPE_MSG_PUMP, RTTHREADFLAGS_WAITABLE,
-                            "Host events");
-        if (RT_FAILURE(rc))
-        {
-            LogRel(("VBoxClient: failed to start seamless event thread, rc=%Rrc.  Disabled seamless capability on host again.\n", rc));
-            VbglR3SeamlessSetCap(false);
-        }
-    }
+        pszStage = "Starting host event thread";
+        rc = startHostEventThread();
+        if (RT_FAILURE(rc))
+            break;
+        pszStage = "Setting guest IRQ filter mask";
+        rc = VbglR3CtlFilterMask(VMMDEV_EVENT_SEAMLESS_MODE_CHANGE_REQUEST, 0);
+        if (RT_FAILURE(rc))
+            break;
+        pszStage = "Reporting support for seamless capability";
+        rc = VbglR3SeamlessSetCap(true);
+        if (RT_FAILURE(rc))
+            break;
+    } while(0);
     if (RT_FAILURE(rc))
     {
-        LogRel(("VBoxClient (seamless): failed to enable seamless capability on host, rc=%Rrc\n", rc));
+        LogRel(("VBoxClient (seamless): failed to start.  Stage: \"%s\"  Error: %Rrc\n",
+                pszStage, rc));
+        stop();
     }
     LogRelFlowFunc(("returning %Rrc\n", rc));
@@ -80 +100 @@
 {
     LogRelFlowFunc(("\n"));
-    if (!mThread)  /* Assertion */
-        LogRel(("VBoxClient: tried to stop seamless service which is not running!\n"));
-    else
-        stopThread();
-    if (mX11MonitorRTThread)
-        stopX11Thread();
+    VbglR3SeamlessSetCap(false);
     VbglR3CtlFilterMask(0, VMMDEV_EVENT_SEAMLESS_MODE_CHANGE_REQUEST);
-    VbglR3SeamlessSetCap(false);
+    if (mHostEventThread)
+        stopHostEventThread();
+    stopX11MonitorThread();
+    mX11Monitor.uninit();
     LogRelFlowFunc(("returning\n"));
 }
@@ -96 +114 @@
  * @returns        IRPT return code.
  */
-int SeamlessMain::nextEvent(void)
+int SeamlessMain::nextStateChangeEvent(void)
 {
     VMMDevSeamlessMode newMode = VMMDev_Seamless_Disabled;
@@ -109 +127 @@
             /* A simplified seamless mode, obtained by making the host VM window borderless and
               making the guest desktop transparent. */
-#ifdef DEBUG
-                LogRelFunc(("VMMDev_Seamless_Visible_Region request received (VBoxClient).\n"));
-#endif
-                mX11ThreadStopping = false;
+                LogRelFlowFunc(("VMMDev_Seamless_Visible_Region request received (VBoxClient).\n"));
                 /** @todo Do something on failure, like bail out. */
-                if (RT_FAILURE(RTThreadCreate(&mX11MonitorRTThread,
-                               x11ThreadFunction, this, 0, RTTHREADTYPE_MSG_PUMP,
-                               RTTHREADFLAGS_WAITABLE, "X11 events")))
-                    LogRelFunc(("Warning: failed to start X11 monitor thread (VBoxClient).\n"));
+                startX11MonitorThread();
                 break;
             case VMMDev_Seamless_Host_Window:
@@ -127 +139 @@
                 /* fall through to case VMMDev_Seamless_Disabled */
             case VMMDev_Seamless_Disabled:
-#ifdef DEBUG
-                LogRelFunc(("VMMDev_Seamless_Disabled set (VBoxClient).\n"));
-#endif
-                if (mX11MonitorRTThread)
-                    stopX11Thread();
-                else
-                    LogRelThisFunc(("Attempted to stop X11 monitor thread which is not running (VBoxClient)!\n"));
+                LogRelFlowFunc(("VMMDev_Seamless_Disabled set (VBoxClient).\n"));
+                stopX11MonitorThread();
         }
     }
@@ -147 +154 @@
  * Update the set of visible rectangles in the host.
  */
-void SeamlessMain::notify(RTRECT *pRects, size_t cRects)
+void SeamlessMain::sendRegionUpdate(RTRECT *pRects, size_t cRects)
 {
     LogRelFlowFunc(("\n"));
@@ -161 +168 @@
 
 /**
- * The actual event thread function.
- */
-int SeamlessMain::threadFunction(RTTHREAD self, void *pvUser)
+ * Thread to listen for seamless state change notifications from the host.
+ */
+int SeamlessMain::hostEventThread(RTTHREAD self, void *pvUser)
 {
     SeamlessMain *pHost = (SeamlessMain *)pvUser;
 
     LogRelFlowFunc(("\n"));
-    pHost->mThreadRunning = true;
+    pHost->mHostEventThreadRunning = true;
     if (0 != pHost)
     {
-        while (!pHost->mThreadStopping)
+        while (!pHost->mHostEventThreadStopping)
         {
-            if (RT_FAILURE(pHost->nextEvent()) && !pHost->mThreadStopping)
+            /* This thread is stopped by setting @a mHostEventThreadStopping
+             * and sending a cancel to the state change event wait, see below.
+             */
+            if (   RT_FAILURE(pHost->nextStateChangeEvent())
+                && !pHost->mHostEventThreadStopping)
             {
                 /* If we are not stopping, sleep for a bit to avoid using up too
@@ -181 +192 @@
         }
     }
-    pHost->mThreadRunning = false;
-    LogRelFlowFunc(("returning VINF_SUCCESS\n"));
+    pHost->mHostEventThreadRunning = false;
     return VINF_SUCCESS;
 }
 
 /**
- * Send a signal to the thread that it should exit
- */
-void SeamlessMain::stopThread()
+ * Start the seamless state change notification listener thread.
+ */
+int SeamlessMain::startHostEventThread()
+{
+    int rc;
+
+    mHostEventThreadStopping = false;
+    rc = RTThreadCreate(&mHostEventThread, hostEventThread, this, 0,
+                        RTTHREADTYPE_MSG_PUMP, RTTHREADFLAGS_WAITABLE,
+                        "Host events");
+    if (RT_FAILURE(rc))
+        LogRel(("VBoxClient: failed to start seamless event thread, rc=%Rrc.\n",
+                rc));
+    return rc;
+}
+
+/**
+ * Send a signal to the host event thread that it should exit and poke it.
+ */
+void SeamlessMain::stopHostEventThread()
 {
     int rc;
@@ -200 +227 @@
      *       yield() should give it time to get to one of places mentioned above.
      */
-    mThreadStopping = true;
-    for (int i = 0; (i < 5) && mThreadRunning; ++i)
+    mHostEventThreadStopping = true;
+    for (int i = 0; (i < 5) && mHostEventThreadRunning; ++i)
     {
         cancelEvent();
         RTThreadYield();
     }
-    rc = RTThreadWait(mThread, RT_INDEFINITE_WAIT, NULL);
+    rc = RTThreadWait(mHostEventThread, RT_INDEFINITE_WAIT, NULL);
     if (RT_SUCCESS(rc))
-        mThread = NIL_RTTHREAD;
+        mHostEventThread = NIL_RTTHREAD;
     else
         LogRelThisFunc(("Failed to stop seamless event thread, rc=%Rrc!\n",
@@ -216 +243 @@
 
 /**
- * The actual X11 event thread function.
- */
-int SeamlessMain::x11ThreadFunction(RTTHREAD self, void *pvUser)
+ * The actual X11 window configuration change monitor thread function.
+ */
+int SeamlessMain::x11MonitorThread(RTTHREAD self, void *pvUser)
 {
     SeamlessMain *pHost = (SeamlessMain *)pvUser;
@@ -227 +254 @@
     if (RT_SUCCESS(rc))
     {
-        while (!pHost->mX11ThreadStopping)
-        {
-            pHost->mX11Monitor.nextEvent();
-        }
+        while (!pHost->mX11MonitorThreadStopping)
+            pHost->mX11Monitor.nextConfigurationEvent();
         pHost->mX11Monitor.stop();
     }
@@ -238 +263 @@
 
 /**
+ * Start the X11 window configuration change monitor thread.
+ */
+int SeamlessMain::startX11MonitorThread(void)
+{
+    int rc;
+
+    mX11MonitorThreadStopping = false;
+    rc = RTThreadCreate(&mX11MonitorThread, x11MonitorThread, this, 0,
+                        RTTHREADTYPE_MSG_PUMP, RTTHREADFLAGS_WAITABLE,
+                        "X11 events");
+    if (RT_FAILURE(rc))
+        LogRelFunc(("Warning: failed to start X11 monitor thread (VBoxClient).\n"));
+    return rc;
+}
+
+/**
  * Send a signal to the thread function that it should exit
  */
-void SeamlessMain::stopX11Thread(void)
-{
-    int rc;
-
-    mX11ThreadStopping = true;
-    mX11Monitor.interruptEvent();
-    rc = RTThreadWait(mX11MonitorRTThread, RT_INDEFINITE_WAIT, NULL);
+void SeamlessMain::stopX11MonitorThread(void)
+{
+    int rc;
+
+    mX11MonitorThreadStopping = true;
+    if (!mX11MonitorThread)
+        return;
+    mX11Monitor.interruptEventWait();
+    rc = RTThreadWait(mX11MonitorThread, RT_INDEFINITE_WAIT, NULL);
     if (RT_SUCCESS(rc))
-        mX11MonitorRTThread = NIL_RTTHREAD;
+        mX11MonitorThread = NIL_RTTHREAD;
     else
         LogRelThisFunc(("Failed to stop X11 monitor thread, rc=%Rrc!\n",
@@ -269 +312 @@
     virtual int run(bool fDaemonised /* = false */)
     {
+        Display *pDisplay = NULL;
+        const char *pszStage;
+        XEvent ev;
         int rc;
 
-        if (mIsInitialised)  /* Assertion */
+        do {
+            pszStage = "Checking that we are not already running";
+            rc = VERR_INTERNAL_ERROR;
+            if (mIsInitialised)  /* Assertion */
+                break;
+            pszStage = "Connecting to the X server";
+            rc = VERR_INTERNAL_ERROR;
+            pDisplay = XOpenDisplay(NULL);
+            if (!pDisplay)
+                break;
+            pszStage = "Starting the service";
+            rc = mSeamless.start();
+            if (RT_FAILURE(rc))
+                break;
+        } while(0);
+        if (RT_FAILURE(rc))
         {
-            LogRelFunc(("error: called a second time! (VBoxClient)\n"));
-            rc = VERR_INTERNAL_ERROR;
-        }
-        if (RT_SUCCESS(rc))
-            rc = mSeamless.init();
-        if (RT_SUCCESS(rc))
-            rc = mSeamless.start();
-        if (RT_SUCCESS(rc))
-            mIsInitialised = true;
-        if (RT_FAILURE(rc))
-        {
-            LogRelFunc(("returning %Rrc (VBoxClient)\n", rc));
+            LogRelFunc(("VBoxClient seamless service control: failed at stage: \"%s\".  Error: %Rrc\n",
+                        pszStage, rc));
+            mSeamless.stop();
+            if (pDisplay)
+                XCloseDisplay(pDisplay);
             return rc;
         }
+        mIsInitialised = true;
         /* Stay running as long as X does... */
-        Display *pDisplay = XOpenDisplay(NULL);
-        XEvent ev;
         while (true)
             XNextEvent(pDisplay, &ev);

trunk/src/VBox/Additions/x11/VBoxClient/seamless.h

@@ -31 +31 @@
 class SeamlessMain : public SeamlessHostProxy
 {
-public:
-    /** Events which can be reported by this class */
-    enum meEvent
-    {
-        /** Empty event */
-        NONE,
-        /** Request to enable seamless mode */
-        ENABLE,
-        /** Request to disable seamless mode */
-        DISABLE
-    };
-
 private:
     // We don't want a copy constructor or assignment operator
@@ -48 +36 @@
     SeamlessMain& operator=(const SeamlessMain&);
 
-    /** Have we been initialised yet? */
-    bool mIsInitialised;
     /** X11 event monitor object */
     SeamlessX11 mX11Monitor;
@@ -55 +41 @@
     /** Thread to start and stop when we enter and leave seamless mode which
      *  monitors X11 windows in the guest. */
-    RTTHREAD mX11MonitorRTThread;
+    RTTHREAD mX11MonitorThread;
     /** Should the X11 monitor thread be stopping? */
-    volatile bool mX11ThreadStopping;
+    volatile bool mX11MonitorThreadStopping;
     /** Host seamless event thread. */
-    RTTHREAD mThread;
+    RTTHREAD mHostEventThread;
     /** Is the thread running? */
-    volatile bool mThreadRunning;
+    volatile bool mHostEventThreadRunning;
     /** Should the thread be stopping? */
-    volatile bool mThreadStopping;
+    volatile bool mHostEventThreadStopping;
 
     /**
@@ -71 +57 @@
      * @returns        IRPT return code.
      */
-    int nextEvent(void);
+    int nextStateChangeEvent(void);
 
     /**
-     * Interrupt an event wait and cause nextEvent() to return immediately.
+     * Interrupt an event wait and cause nextStateChangeEvent() to return immediately.
      */
     void cancelEvent(void) { VbglR3InterruptEventWaits(); }
@@ -80 +66 @@
     /** Thread function to query seamless activation and deactivation events
      *  from the host. */
-    static DECLCALLBACK(int) threadFunction(RTTHREAD self, void *pvUser);
+    static DECLCALLBACK(int) hostEventThread(RTTHREAD self, void *pvUser);
+
+    /** Helper to start the seamless state change notification listener
+     * thread. */
+    int startHostEventThread();
 
     /** Helper to stop the event query thread again. */
-    void stopThread();
+    void stopHostEventThread();
 
     /** Thread function to monitor X11 window configuration changes. */
-    static DECLCALLBACK(int) x11ThreadFunction(RTTHREAD self, void *pvUser);
+    static DECLCALLBACK(int) x11MonitorThread(RTTHREAD self, void *pvUser);
+
+    /** Helper to start the X11 monitor thread. */
+    int startX11MonitorThread(void);
 
     /** Helper to stop the X11 monitor thread again. */
-    void stopX11Thread(void);
+    void stopX11MonitorThread(void);
 
 public:
-    /**
-     * Initialise the guest and ensure that it is capable of handling seamless mode
-     * @param   pX11Monitor Object to monitor X11 guest windows.
-     *
-     * @returns iprt status code
-     */
-    int init(void)
-    {
-        int rc;
-
-        LogRelFlowFunc(("\n"));
-        if (mIsInitialised)
-            return VERR_INTERNAL_ERROR;
-        rc = mX11Monitor.init(this);
-        if (RT_SUCCESS(rc))
-            mIsInitialised = true;
-        return rc;
-    }
+    SeamlessMain(void);
+    ~SeamlessMain();
 
     /**
@@ -119 +96 @@
     /**
      * Stops the service.
-     * @param cMillies how long to wait for the thread to exit
      */
     void stop();
@@ -126 +102 @@
      * Update the set of visible rectangles in the host.
      */
-    virtual void notify(RTRECT *pRects, size_t cRects);
-
-    SeamlessMain(void)
-    {
-        mIsInitialised = false;
-        mX11MonitorRTThread = NIL_RTTHREAD;
-        mX11ThreadStopping = false;
-        mThread = NIL_RTTHREAD;
-        mThreadRunning = false;
-        mThreadStopping = false;
-    }
-
-    ~SeamlessMain()
-    {
-        LogRelFlowFunc(("\n"));
-        if (mThread)
-            stop();
-        LogRelFlowFunc(("returning\n"));
-    }
+    virtual void sendRegionUpdate(RTRECT *pRects, size_t cRects);
 };
 

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

@@ -303 +303 @@
 public:
     testHost() : mfNotified(false) {}
-    virtual void notify(RTRECT *pRects, size_t cRects)
+    virtual void sendRegionUpdate(RTRECT *pRects, size_t cRects)
     {
         mfNotified = true;
@@ -625 +625 @@
         ++cErrs;
     }
-    subject.nextEvent();
+    subject.nextConfigurationEvent();
     if (!host.isNotified())
     {
@@ -633 +633 @@
     }
     smlsSetNextEvent(0, 0);
-    subject.nextEvent();
+    subject.nextConfigurationEvent();
     if (!host.isNotified())
     {

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

@@ -35 +35 @@
                  pRects[i].yBottom);
     }
-    return true;
+    return VINF_SUCCESS;
 }
 
@@ -42 +42 @@
     RTPrintf("%s\n", bState ? "Seamless capability set"
                             : "Seamless capability unset");
-    return true;
+    return VINF_SUCCESS;
 }
 
@@ -49 +49 @@
     RTPrintf("IRQ filter mask changed.  Or mask: 0x%x.  Not mask: 0x%x\n",
              u32OrMask, u32NotMask);
-    return true;
+    return VINF_SUCCESS;
 }
 
@@ -70 +70 @@
         }
     }
-    return true;
+    return rc;
 }
 
@@ -121 +121 @@
     SeamlessMain seamless;
     LogRel(("Starting seamless Guest Additions...\n"));
-    rc = seamless.init();
+    rc = seamless.start();
     if (rc != VINF_SUCCESS)
     {
-        RTPrintf("Failed to initialise seamless Additions, rc = %d\n", rc);
+        RTPrintf("Failed to initialise seamless Additions, rc = %Rrc\n", rc);
     }
     RTStrmGetLine(g_pStdIn, ach, sizeof(ach));