From 6bf31327c69ad6aacccb6aefd1b1f7b202413ae6 Mon Sep 17 00:00:00 2001
From: Matthias Bolte <matthias.bolte@googlemail.com>
Date: Sun, 1 May 2011 21:57:43 +0200
Subject: [PATCH] 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.
---
 src/esx/esx_vi.c | 47 +++++++++++++++++++++++++++++++++++------------
 src/esx/esx_vi.h |  4 +++-
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 381b547cf0..10889d0b37 100644
--- 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 75469abcbc..e3d096ef4e 100644
--- 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;