Changeset 83489 in vbox for trunk/src/VBox/Main/src-client

Timestamp:

Mar 30, 2020 4:38:23 PM (5 years ago)

Author:

vboxsync

Message:

Guest Control/Main: Fixed IGuestSession::directoryOpen() API to fail if a guest directory does not exist . bugref:9320

Location:

trunk/src/VBox/Main/src-client

Files:

• GuestDirectoryImpl.cpp (modified) (7 diffs)
• GuestSessionImpl.cpp (modified) (2 diffs)
• GuestSessionImplTasks.cpp (modified) (1 diff)

trunk/src/VBox/Main/src-client/GuestDirectoryImpl.cpp

@@ -83 +83 @@
         /* Start the directory process on the guest. */
         GuestProcessStartupInfo procInfo;
-        procInfo.mName      = Utf8StrFmt(tr("Reading directory \"%s\""), openInfo.mPath.c_str());
+        procInfo.mName      = Utf8StrFmt(tr("Opening directory \"%s\""), openInfo.mPath.c_str());
         procInfo.mTimeoutMS = 5 * 60 * 1000; /* 5 minutes timeout. */
         procInfo.mFlags     = ProcessCreateFlag_WaitForStdOut;
@@ -100 +100 @@
 
         /*
-         * Start the process asynchronously and keep it around so that we can use
+         * Start the process synchronously and keep it around so that we can use
          * it later in subsequent read() calls.
-         * Note: No guest rc available because operation is asynchronous.
          */
-        vrc = mData.mProcessTool.init(mSession, procInfo, true /* Async */, NULL /* Guest rc */);
-
-/** @todo r=bird: IGuest::directoryOpen need to fail if the directory doesn't
- *        exist like it is documented to do.  It seems this async approach or
- *        something is delaying such errors till GuestDirectory::read() is
- *        called, which is clearly messed up.
- */
+        vrc = mData.mProcessTool.init(mSession, procInfo, false /* Async */, NULL /* Guest rc */);
+        if (RT_SUCCESS(vrc))
+        {
+            /* As we need to know if the directory we were about to open exists and and is accessible,
+             * do the first read here in order to return a meaningful status here. */
+            int rcGuest = VERR_IPE_UNINITIALIZED_STATUS;
+            vrc = i_readInternal(mData.mObjData, &rcGuest);
+            if (RT_FAILURE(vrc))
+            {
+                /*
+                 * We need to actively terminate our process tool in case of an error here,
+                 * as this otherwise would be done on (directory) object destruction implicitly.
+                 * This in turn then will run into a timeout, as the directory object won't be
+                 * around anymore at that time. Ugly, but that's how it is for the moment.
+                 */
+                int vrcTerm = mData.mProcessTool.terminate(30 * RT_MS_1SEC, NULL /* prcGuest */);
+                AssertRC(vrcTerm);
+
+                if (vrc == VERR_GSTCTL_GUEST_ERROR)
+                    vrc = rcGuest;
+            }
+        }
     }
 
@@ -118 +132 @@
     else
         autoInitSpan.setFailed();
+
+    LogFlowFuncLeaveRC(vrc);
     return vrc;
 }
@@ -284 +300 @@
 
 /**
- * Reads the next directory entry.
+ * Reads the next directory entry, internal version.
  *
  * @return VBox status code. Will return VERR_NO_MORE_FILES if no more entries are available.
- * @param  fsObjInfo            Where to store the read directory entry.
+ * @param  objData              Where to store the read directory entry as internal object data.
  * @param  prcGuest             Where to store the guest result code in case VERR_GSTCTL_GUEST_ERROR is returned.
  */
-int GuestDirectory::i_readInternal(ComObjPtr<GuestFsObjInfo> &fsObjInfo, int *prcGuest)
+int GuestDirectory::i_readInternal(GuestFsObjData &objData, int *prcGuest)
 {
     AssertPtrReturn(prcGuest, VERR_INVALID_POINTER);
-
-    /* Create the FS info object. */
-    HRESULT hr = fsObjInfo.createObject();
-    if (FAILED(hr))
-        return VERR_COM_UNEXPECTED;
 
     GuestProcessStreamBlock curBlock;
@@ -314 +325 @@
             if (curBlock.GetCount()) /* Did we get content? */
             {
-                GuestFsObjData objData;
-                rc = objData.FromLs(curBlock, true /* fLong */);
-                if (RT_SUCCESS(rc))
+                if (curBlock.GetString("name"))
                 {
-                   rc = fsObjInfo->init(objData);
+                    rc = objData.FromLs(curBlock, true /* fLong */);
                 }
                 else
@@ -335 +344 @@
 }
 
+/**
+ * Reads the next directory entry.
+ *
+ * @return VBox status code. Will return VERR_NO_MORE_FILES if no more entries are available.
+ * @param  fsObjInfo            Where to store the read directory entry.
+ * @param  prcGuest             Where to store the guest result code in case VERR_GSTCTL_GUEST_ERROR is returned.
+ */
+int GuestDirectory::i_read(ComObjPtr<GuestFsObjInfo> &fsObjInfo, int *prcGuest)
+{
+    AssertPtrReturn(prcGuest, VERR_INVALID_POINTER);
+
+    /* Create the FS info object. */
+    HRESULT hr = fsObjInfo.createObject();
+    if (FAILED(hr))
+        return VERR_COM_UNEXPECTED;
+
+    int rc;
+
+    /* If we have a valid object data cache, read from it. */
+    if (mData.mObjData.mName.isNotEmpty())
+    {
+        rc = fsObjInfo->init(mData.mObjData);
+        if (RT_SUCCESS(rc))
+        {
+            mData.mObjData.mName = ""; /* Mark the object data as being empty (beacon). */
+        }
+    }
+    else /* Otherwise ask the guest for the next object data (block). */
+    {
+
+        GuestFsObjData objData;
+        rc = i_readInternal(objData, prcGuest);
+        if (RT_SUCCESS(rc))
+            rc = fsObjInfo->init(objData);
+    }
+
+    LogFlowThisFunc(("Returning rc=%Rrc\n", rc));
+    return rc;
+}
+
 /* static */
 HRESULT GuestDirectory::i_setErrorExternal(VirtualBoxBase *pInterface, int rcGuest)
@@ -391 +440 @@
     ComObjPtr<GuestFsObjInfo> fsObjInfo;
     int rcGuest = VERR_IPE_UNINITIALIZED_STATUS;
-    int vrc = i_readInternal(fsObjInfo, &rcGuest);
+    int vrc = i_read(fsObjInfo, &rcGuest);
     if (RT_SUCCESS(vrc))
     {

trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp

@@ -1165 +1165 @@
     }
 
+    /* We need to release the write lock first before initializing the directory object below,
+     * as we're starting a guest process as part of it. This in turn will try to acquire the session's
+     * write lock. */
+    alock.release();
+
     Console *pConsole = mParent->i_getConsole();
     AssertPtr(pConsole);
@@ -1170 +1175 @@
     vrc = pDirectory->init(pConsole, this /* Parent */, idObject, openInfo);
     if (RT_FAILURE(vrc))
-        return vrc;
-
-    /*
-     * Since this is a synchronous guest call we have to
-     * register the file object first, releasing the session's
-     * lock and then proceed with the actual opening command
-     * -- otherwise the file's opening callback would hang
-     * because the session's lock still is in place.
-     */
-    try
-    {
-        /* Add the created directory to our map. */
-        mData.mDirectories[idObject] = pDirectory;
-
-        LogFlowFunc(("Added new guest directory \"%s\" (Session: %RU32) (now total %zu directories)\n",
-                     openInfo.mPath.c_str(), mData.mSession.mID, mData.mDirectories.size()));
-
-        alock.release(); /* Release lock before firing off event. */
-
-        /** @todo Fire off a VBoxEventType_OnGuestDirectoryRegistered event? */
-    }
-    catch (std::bad_alloc &)
-    {
-        vrc = VERR_NO_MEMORY;
+    {
+        /* Make sure to acquire the write lock again before unregistering the object. */
+        alock.acquire();
+
+        int vrc2 = i_objectUnregister(idObject);
+        AssertRC(vrc2);
+
+        pDirectory.setNull();
+    }
+    else
+    {
+        /* Make sure to acquire the write lock again before continuing. */
+        alock.acquire();
+
+        try
+        {
+            /* Add the created directory to our map. */
+            mData.mDirectories[idObject] = pDirectory;
+
+            LogFlowFunc(("Added new guest directory \"%s\" (Session: %RU32) (now total %zu directories)\n",
+                         openInfo.mPath.c_str(), mData.mSession.mID, mData.mDirectories.size()));
+
+            alock.release(); /* Release lock before firing off event. */
+
+            /** @todo Fire off a VBoxEventType_OnGuestDirectoryRegistered event? */
+        }
+        catch (std::bad_alloc &)
+        {
+            vrc = VERR_NO_MEMORY;
+        }
     }
 

trunk/src/VBox/Main/src-client/GuestSessionImplTasks.cpp

@@ -1110 +1110 @@
     {
         ComObjPtr<GuestFsObjInfo> fsObjInfo;
-        while (RT_SUCCESS(rc = pDir->i_readInternal(fsObjInfo, &rcGuest)))
+        while (RT_SUCCESS(rc = pDir->i_read(fsObjInfo, &rcGuest)))
         {
             FsObjType_T enmObjType = FsObjType_Unknown; /* Shut up MSC. */