Hgfs Server: Fix shares update for change notification

This change fixes UI updates of the shares which currently
update by a poorly designed algorithm which causes change
notification subscriptions to be deleted and not restored
and also unnecessary deactivation and activation of the
events generation.

Currently, the UI updates the HGFS shares in the HGFS server
via the RegisterShare callback. This is done from HGFS VMX
policy code which handles updates from the UI and maintains
the HGFS shares. The UI calls the policy code for each share
to remove it until the list of shares is empty. It then calls
back for each share to add the existing shares back to the list.
Currently, the policy code calls the HGFS server RegisterShare
callback to delete the share as it removes them and then again
to add each one back in that added to the policy shares list.
Unfortunately, as the shares are removed any existing subscribers
for change notifications on the deleted share are also destroyed.
Furthermore, when the shares list is empty, the count of subscribers
will also be zero at this point causing a deactivation of the
file change event generator. Once, the shares are added back in
via the RegisterShare callback no event subscribers will be
restored as they are not tracked and the event generation
will also not be activated due to that reason.

The fix is to make the shares tracking be handled in the same
manner as the HGFS server does for open handles on shares.
The HGFS policy waits until the shares list updates are complete
as the UI is done updating the shares. The HGFS policy then calls
the HGFS server InvalidateObjects callback with the new complete
list of shares which may or may not be different from the previous
list. The HGFS server InvalidateObjects goes through its current list
of shares and searches for each share in the new list of shares and
if it fails to locate it, then closes all open handles on that share.
Once the list has been exhausted, all open handles remaining will be
on shares that still exist.

The same callback is now used for resetting the list of shares for
the current subscribers of change events and not the RegisterShare
callback which should be removed altogether.  We now make sure that
the reset of file change event shares only removes shares not found
on the new list thereby only removing subscribers on those stale shares.
All subscribers for shares remaining on the lists are not deleted,
remain intact and unaffected.  Furthermore, file system event
generation will also not be deactivated unless all subscribers
happen to be for shares that were deleted.

This change is the first part which as code to piggyback on the HGFS
server InvalidateObjects callback to compare any shares in its list
with the new list and delete shares not found.  The RegisterSharecallback
function code is deleted and temporarily is an empty callback.
This will be deleted along with the callback code from the HGFS
policy manager in a subsequent change.

Details:
HgfsServerRegisterShare - is gutted and just an empty function just
   logging that it is called.
HgfsServerCleanupDeletedFolders is effectively renamed to
   HgfsServerSharesDeleteStale w/o the lock acquistion/release.
HgfsServerSharesDeleteStale now free the share name as well as the
   share object to stop memory leak.
HgfsServerShareAddInternal is extracted from the old defunct
   HgfsServerRegisterShare w/o the lock acquisition/release.
HgfsServerShareAdd acquires and releases the lock and calls
   HgfsServerShareAddInternal
HgfsServerSharesReset is created and called from the
   HgfsServerInvalidateObjects
HgfsServerEnumerateSharedFolders now calls the new HgfsServerShareAdd
   for all shares which is used to initialize the shares on HGFS server
   start.
HgfsServerGetLocalNameInfo now does not get the Shared Folder handle
   from the VMX policy host, instead getting it from the existing
   HgfsServerGetShareHandle (the server maintained list of shares).
   This is only used for setting subscribers from the client when
   using handles and not path names.
HgfsServerSetDirWatchByName now uses the handle returned by the
   HgfsServerGetLocalNameInfo call.
markedForDeletion is now removed as holding stale state is not needed
   since stale shares are deleted and resources freed immediately.

diff --git a/open-vm-tools/lib/hgfsServer/hgfsServer.c b/open-vm-tools/lib/hgfsServer/hgfsServer.c
index ae9b29f90ae5ecdb5f71dab6bdfb85da46061c37..22bc566f8a76e935e96949cea4d226e1185c8f2f 100644 (file)
--- a/open-vm-tools/lib/hgfsServer/hgfsServer.c
+++ b/open-vm-tools/lib/hgfsServer/hgfsServer.c
@@ -273,7 +273,6 @@ typedef struct HgfsSharedFolderProperties {
    DblLnkLst_Links links;
    char *name;                                /* Name of the share. */
    HgfsSharedFolderHandle notificationHandle; /* Directory notification handle. */
-   Bool markedForDeletion;
 } HgfsSharedFolderProperties;
 
 
@@ -3583,12 +3582,14 @@ abort:
 /*
  *-----------------------------------------------------------------------------
  *
- * HgfsServerCleanupDeletedFolders --
+ * HgfsServerSharesDeleteStale --
  *
  *    This function iterates through all shared folders and removes all
  *    deleted shared folders, removes them from notification package and
  *    from the folders list.
  *
+ *    Note: this assumes that the list lock is already acquired.
+ *
  * Results:
  *    None.
  *
@@ -3599,26 +3600,200 @@ abort:
  */
 
 static void
-HgfsServerCleanupDeletedFolders(void)
+HgfsServerSharesDeleteStale(DblLnkLst_Links *newSharesList)  // IN: new list of shares
 {
    DblLnkLst_Links *link, *nextElem;
 
-   MXUser_AcquireExclLock(gHgfsSharedFoldersLock);
    DblLnkLst_ForEachSafe(link, nextElem, &gHgfsSharedFoldersList) {
-      HgfsSharedFolderProperties *folder =
+      HgfsSharedFolderProperties *currentShare =
          DblLnkLst_Container(link, HgfsSharedFolderProperties, links);
-      if (folder->markedForDeletion) {
+      Bool staleShare = TRUE;
+      DblLnkLst_Links *linkNewShare, *nextNewShare;
+
+      DblLnkLst_ForEachSafe(linkNewShare, nextNewShare, newSharesList) {
+         HgfsSharedFolder *newShare =
+            DblLnkLst_Container(linkNewShare, HgfsSharedFolder, links);
+
+         ASSERT(newShare);
+
+         if (strcmp(currentShare->name, newShare->name) == 0) {
+            LOG(4, ("%s: %s is still valid\n", __FUNCTION__, newShare->name));
+            staleShare = FALSE;
+            break;
+         }
+      }
+
+      if (staleShare) {
          LOG(8, ("%s: removing shared folder handle %#x\n",
-                 __FUNCTION__, folder->notificationHandle));
-         if (!HgfsNotify_RemoveSharedFolder(folder->notificationHandle)) {
-            LOG(4, ("Problem removing %d shared folder handle\n",
-                    folder->notificationHandle));
+                 __FUNCTION__, currentShare->notificationHandle));
+         if (!HgfsNotify_RemoveSharedFolder(currentShare->notificationHandle)) {
+            LOG(4, ("%s: Error: removing %d shared folder handle\n",
+                    __FUNCTION__, currentShare->notificationHandle));
          }
          DblLnkLst_Unlink1(link);
-         free(folder);
+         free(currentShare->name);
+         free(currentShare);
+      }
+   }
+}
+
+
+/*
+ *-----------------------------------------------------------------------------
+ *
+ * HgfsServerShareAddInternal --
+ *
+ *    Add a new shared folder property entry if it is not in the list of shares.
+ *
+ *    Note: this assumes that the list lock is already acquired.
+ *
+ * Results:
+ *    The share handle if entry was found HGFS_INVALID_FOLDER_HANDLE if not.
+ *
+ * Side effects:
+ *    May add a shared folders entry for change notifications.
+ *
+ *-----------------------------------------------------------------------------
+ */
+
+static HgfsSharedFolderHandle
+HgfsServerShareAddInternal(const char *shareName,   // IN: shared folder name
+                           const char *sharePath)   // IN: shared folder path)
+{
+   HgfsSharedFolderHandle handle = HGFS_INVALID_FOLDER_HANDLE;
+   DblLnkLst_Links *link, *nextElem;
+
+   DblLnkLst_ForEachSafe(link, nextElem, &gHgfsSharedFoldersList) {
+      HgfsSharedFolderProperties *currentShare =
+         DblLnkLst_Container(link, HgfsSharedFolderProperties, links);
+
+      ASSERT(currentShare);
+
+      if (strcmp(currentShare->name, shareName) == 0) {
+         LOG(8, ("%s: Share is not new\n", __FUNCTION__));
+         handle = currentShare->notificationHandle;
+         break;
+      }
+   }
+
+   /* If the share is new then add it to the notification shares. */
+   if (handle == HGFS_INVALID_FOLDER_HANDLE) {
+      handle = HgfsNotify_AddSharedFolder(sharePath, shareName);
+      if (HGFS_INVALID_FOLDER_HANDLE != handle) {
+         HgfsSharedFolderProperties *shareProps =
+            (HgfsSharedFolderProperties *)Util_SafeMalloc(sizeof *shareProps);
+
+         shareProps->notificationHandle = handle;
+         shareProps->name = Util_SafeStrdup(shareName);
+         DblLnkLst_Init(&shareProps->links);
+         DblLnkLst_LinkLast(&gHgfsSharedFoldersList, &shareProps->links);
       }
+
+      LOG(8, ("%s: %s, %s, add hnd %#x\n",__FUNCTION__,
+               (shareName ? shareName : "NULL"),
+               (sharePath ? sharePath : "NULL"),
+               handle));
+   }
+   return handle;
+}
+
+
+/*
+ *-----------------------------------------------------------------------------
+ *
+ * HgfsServerShareAdd --
+ *
+ *    Add a new shared folder property entry if it is not in the list of shares.
+ *
+ *    Note: this assumes that the list lock is already acquired.
+ *
+ * Results:
+ *    The shares handle if found or added. HGFS_INVALID_FOLDER_HANDLE otherwise.
+ *
+ * Side effects:
+ *    May add a shared folders entry for change notifications.
+ *
+ *-----------------------------------------------------------------------------
+ */
+
+static HgfsSharedFolderHandle
+HgfsServerShareAdd(const char *shareName,   // IN: shared folder name
+                   const char *sharePath)   // IN: shared folder path)  // IN: List of new shares
+{
+   HgfsSharedFolderHandle handle;
+
+   LOG(8, ("%s: entered\n", __FUNCTION__));
+
+   if (!gHgfsDirNotifyActive) {
+      LOG(8, ("%s: notification disabled\n", __FUNCTION__));
+      return HGFS_INVALID_FOLDER_HANDLE;
    }
+
+   MXUser_AcquireExclLock(gHgfsSharedFoldersLock);
+   handle = HgfsServerShareAddInternal(shareName, sharePath);
    MXUser_ReleaseExclLock(gHgfsSharedFoldersLock);
+
+   LOG(8, ("%s: exit(%#x)\n", __FUNCTION__, handle));
+   return handle;
+}
+
+
+/*
+ *-----------------------------------------------------------------------------
+ *
+ * HgfsServerSharesReset --
+ *
+ *    This is a callback function which is invoked by hgfsServerManagement
+ *    for every shared folder when something changed in shared folders
+ *    configuration.
+ *    The function any entries now not present as stale and removes them.
+ *    Any entries on the new list of shares not on the list of list of shares
+ *    will have new entries created and added to the list.
+ *
+ * Results:
+ *    None.
+ *
+ * Side effects:
+ *    May add an entry to known shared folders list.
+ *
+ *-----------------------------------------------------------------------------
+ */
+
+static void
+HgfsServerSharesReset(DblLnkLst_Links *newSharesList)  // IN: List of new shares
+{
+   DblLnkLst_Links *linkNewShare, *nextNewShare;
+
+   LOG(8, ("%s: entered\n", __FUNCTION__));
+
+   if (!gHgfsDirNotifyActive) {
+      LOG(8, ("%s: notification disabled\n", __FUNCTION__));
+      return;
+   }
+
+   MXUser_AcquireExclLock(gHgfsSharedFoldersLock);
+
+   /*
+    * Now we go through the shares properties list to
+    * remove and delete those shares that are stale.
+    */
+   HgfsServerSharesDeleteStale(newSharesList);
+
+   /*
+    * Iterate over the new shares and check for any not in the updated shares properties
+    * list, as those will need a new share property created and added to the list.
+    */
+   DblLnkLst_ForEachSafe(linkNewShare, nextNewShare, newSharesList) {
+      HgfsSharedFolder *newShare =
+         DblLnkLst_Container(linkNewShare, HgfsSharedFolder, links);
+
+      ASSERT(newShare);
+
+      HgfsServerShareAddInternal(newShare->name, newShare->path);
+   }
+
+   MXUser_ReleaseExclLock(gHgfsSharedFoldersLock);
+   LOG(8, ("%s: exit\n", __FUNCTION__));
 }
 
 
@@ -3655,58 +3830,7 @@ HgfsServerRegisterShare(const char *shareName,   // IN: shared folder name
                         const char *sharePath,   // IN: shared folder path
                         Bool addFolder)          // IN: add or remove folder
 {
-   DblLnkLst_Links *link, *nextElem;
    HgfsSharedFolderHandle result = HGFS_INVALID_FOLDER_HANDLE;
-
-   LOG(8, ("%s: %s, %s, %s\n", __FUNCTION__,
-           (shareName ? shareName : "NULL"), (sharePath ? sharePath : "NULL"),
-           (addFolder ? "add" : "remove")));
-
-   if (!gHgfsDirNotifyActive) {
-      LOG(8, ("%s: notification disabled\n", __FUNCTION__));
-      goto exit;
-   }
-
-   LOG(8, ("%s: %s, %s, %s - active notification\n", __FUNCTION__,
-           (shareName ? shareName : "NULL"), (sharePath ? sharePath : "NULL"),
-           (addFolder ? "add" : "remove")));
-
-   if (NULL == shareName) {
-      /*
-       * The function is invoked with shareName NULL when all shares has been
-       * enumerated.
-       * Need to delete all shared folders that were marked for deletion.
-       */
-      HgfsServerCleanupDeletedFolders();
-      goto exit;
-   }
-
-   MXUser_AcquireExclLock(gHgfsSharedFoldersLock);
-
-   DblLnkLst_ForEachSafe(link, nextElem, &gHgfsSharedFoldersList) {
-      HgfsSharedFolderProperties *folder =
-         DblLnkLst_Container(link, HgfsSharedFolderProperties, links);
-      if (strcmp(folder->name, shareName) == 0) {
-         result = folder->notificationHandle;
-         folder->markedForDeletion = !addFolder;
-         break;
-      }
-   }
-   if (addFolder && HGFS_INVALID_FOLDER_HANDLE == result) {
-      result = HgfsNotify_AddSharedFolder(sharePath, shareName);
-      if (HGFS_INVALID_FOLDER_HANDLE != result) {
-         HgfsSharedFolderProperties *folder =
-            (HgfsSharedFolderProperties *)Util_SafeMalloc(sizeof *folder);
-         folder->notificationHandle = result;
-         folder->name = Util_SafeStrdup(shareName);
-         folder->markedForDeletion = FALSE;
-         DblLnkLst_Init(&folder->links);
-         DblLnkLst_LinkLast(&gHgfsSharedFoldersList, &folder->links);
-      }
-   }
-   MXUser_ReleaseExclLock(gHgfsSharedFoldersLock);
-
-exit:
    LOG(8, ("%s: %s, %s, %s exit %#x\n",__FUNCTION__,
            (shareName ? shareName : "NULL"), (sharePath ? sharePath : "NULL"),
            (addFolder ? "add" : "remove"), result));
@@ -4202,7 +4326,7 @@ HgfsServerEnumerateSharedFolders(void)
                                                        &sharePathLen, &sharePath);
             if (HGFS_NAME_STATUS_COMPLETE == nameStatus) {
                LOG(8, ("%s: registering share %s path %s\n", __FUNCTION__, shareName, sharePath));
-               handle = HgfsServerRegisterShare(shareName, sharePath, TRUE);
+               handle = HgfsServerShareAdd(shareName, sharePath);
                success = handle != HGFS_INVALID_FOLDER_HANDLE;
                LOG(8, ("%s: registering share %s hnd %#x\n", __FUNCTION__, shareName, handle));
             }
@@ -4226,7 +4350,7 @@ HgfsServerEnumerateSharedFolders(void)
  *    Allocate HgfsTransportSessionInfo and initialize it.
  *
  * Results:
- *    TRUE on success, FALSE otherwise.
+ *    TRUE always.
  *
  * Side effects:
  *    None.
@@ -5028,6 +5152,8 @@ HgfsServerSessionInvalidateObjects(void *clientData,         // IN:
    HgfsTransportSessionInfo *transportSession = clientData;
    DblLnkLst_Links *curr;
 
+   LOG(4, ("%s: Beginning\n", __FUNCTION__));
+
    ASSERT(transportSession);
    MXUser_AcquireExclLock(transportSession->sessionArrayLock);
 
@@ -5039,6 +5165,10 @@ HgfsServerSessionInvalidateObjects(void *clientData,         // IN:
    }
 
    MXUser_ReleaseExclLock(transportSession->sessionArrayLock);
+
+   /* Now invalidate any stale shares and add any new ones. */
+   HgfsServerSharesReset(shares);
+   LOG(4, ("%s: Ending\n", __FUNCTION__));
 }
 
 
@@ -5259,13 +5389,22 @@ HgfsServerGetLocalNameInfo(const char *cpName,      // IN:  Cross-platform filen
                                                len,
                                                &shareInfo->readPermissions,
                                                &shareInfo->writePermissions,
-                                               &shareInfo->handle,
+                                               &shareInfo->handle, // XXX: to be deleted.
                                                &shareInfo->rootDir);
    if (nameStatus != HGFS_NAME_STATUS_COMPLETE) {
       LOG(4, ("%s: No such share (%s)\n", __FUNCTION__, cpName));
       return nameStatus;
    }
    shareInfo->rootDirLen = strlen(shareInfo->rootDir);
+   /*
+    * XXX: The handle is now NOT propagated back and held in the policy but only in the
+    * table of share properties.
+    *
+    * Get shareInfo handle returns a valid handle only if we have change
+    * notification active.
+    * Note: cpName begins with the share name.
+    */
+   shareInfo->handle = HgfsServerGetShareHandle(cpName);
 
    /* Get the config options. */
    nameStatus = HgfsServerPolicy_GetShareOptions(cpName, len, &shareOptions);
@@ -7657,7 +7796,7 @@ HgfsServerSetDirWatchByName(HgfsInputParam *input,         // IN: Input params
          /* See if we are dealing with the base of the namespace */
          nameStatus = HGFS_NAME_STATUS_INCOMPLETE_BASE;
       } else {
-         sharedFolder = HgfsServerGetShareHandle(cpName);
+         sharedFolder = shareInfo.handle;
       }
 
       if (HGFS_NAME_STATUS_COMPLETE == nameStatus &&