esx: Fix race condition in esxVI_EnsureSession

When the session has expired then multiple threads can race while
reestablishing it.

This race condition is not that critical as it requires a special usage
pattern to be triggered. It can only happen when an application doesn't
do API calls for quite some time (the session expires after 30 min
inactivity) and then multiple threads doing simultaneous API calls and
end up doing simultaneous calls to esxVI_EnsureSession.

diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 381b547cf0957a100067d3c37e9d6bccc2908f25..10889d0b37ce14b6e48fb76ca3cea5de3f2814af 100644 (file)
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -603,6 +603,10 @@ ESX_VI__TEMPLATE__ALLOC(Context)
 /* esxVI_Context_Free */
 ESX_VI__TEMPLATE__FREE(Context,
 {
+    if (item->sessionLock != NULL) {
+        virMutexDestroy(item->sessionLock);
+    }
+
     esxVI_CURL_Free(&item->curl);
     VIR_FREE(item->url);
     VIR_FREE(item->ipAddress);
@@ -610,6 +614,7 @@ ESX_VI__TEMPLATE__FREE(Context,
     VIR_FREE(item->password);
     esxVI_ServiceContent_Free(&item->service);
     esxVI_UserSession_Free(&item->session);
+    VIR_FREE(item->sessionLock);
     esxVI_Datacenter_Free(&item->datacenter);
     esxVI_ComputeResource_Free(&item->computeResource);
     esxVI_HostSystem_Free(&item->hostSystem);
@@ -642,6 +647,17 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url,
         return -1;
     }
 
+    if (VIR_ALLOC(ctx->sessionLock) < 0) {
+        virReportOOMError();
+        return -1;
+    }
+
+    if (virMutexInit(ctx->sessionLock) < 0) {
+        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
+                     _("Could not initialize session mutex"));
+        return -1;
+    }
+
     if (esxVI_RetrieveServiceContent(ctx, &ctx->service) < 0) {
         return -1;
     }
@@ -1548,11 +1564,18 @@ esxVI_EnsureSession(esxVI_Context *ctx)
     esxVI_DynamicProperty *dynamicProperty = NULL;
     esxVI_UserSession *currentSession = NULL;
 
-    if (ctx->session == NULL) {
-        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call"));
+    if (ctx->sessionLock == NULL) {
+        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call, no mutex"));
         return -1;
     }
 
+    virMutexLock(ctx->sessionLock);
+
+    if (ctx->session == NULL) {
+        ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call, no session"));
+        goto cleanup;
+    }
+
     if (ctx->hasSessionIsActive) {
         /*
          * Use SessionIsActive to check if there is an active session for this
@@ -1560,7 +1583,7 @@ esxVI_EnsureSession(esxVI_Context *ctx)
          */
         if (esxVI_SessionIsActive(ctx, ctx->session->key,
                                   ctx->session->userName, &active) < 0) {
-            return -1;
+            goto cleanup;
         }
 
         if (active != esxVI_Boolean_True) {
@@ -1568,11 +1591,9 @@ esxVI_EnsureSession(esxVI_Context *ctx)
 
             if (esxVI_Login(ctx, ctx->username, ctx->password, NULL,
                             &ctx->session) < 0) {
-                return -1;
+                goto cleanup;
             }
         }
-
-        return 0;
     } else {
         /*
          * Query the session manager for the current session of this connection
@@ -1614,16 +1635,18 @@ esxVI_EnsureSession(esxVI_Context *ctx)
                            "last login"));
             goto cleanup;
         }
+    }
 
-        result = 0;
+    result = 0;
 
   cleanup:
-        esxVI_String_Free(&propertyNameList);
-        esxVI_ObjectContent_Free(&sessionManager);
-        esxVI_UserSession_Free(&currentSession);
+    virMutexUnlock(ctx->sessionLock);
 
-        return result;
-    }
+    esxVI_String_Free(&propertyNameList);
+    esxVI_ObjectContent_Free(&sessionManager);
+    esxVI_UserSession_Free(&currentSession);
+
+    return result;
 }
 
 

diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h
index 75469abcbcb26f218f34b16c9e4f41c1faa7ecf6..e3d096ef4e7b29a36e5c1d408180bbdf6117f509 100644 (file)
--- a/src/esx/esx_vi.h
+++ b/src/esx/esx_vi.h
@@ -185,6 +185,7 @@ int esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl);
  */
 
 struct _esxVI_Context {
+    /* All members are used read-only after esxVI_Context_Connect ... */
     esxVI_CURL *curl;
     char *url;
     char *ipAddress;
@@ -193,7 +194,8 @@ struct _esxVI_Context {
     esxVI_ServiceContent *service;
     esxVI_APIVersion apiVersion;
     esxVI_ProductVersion productVersion;
-    esxVI_UserSession *session;
+    esxVI_UserSession *session; /* ... except the session ... */
+    virMutexPtr sessionLock; /* ... that is protected by this mutex */
     esxVI_Datacenter *datacenter;
     esxVI_ComputeResource *computeResource;
     esxVI_HostSystem *hostSystem;