Add copy and move constructors to AWSClient

addresses the issue raised in #815

Author: Marco Magdy

aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp

@@ -29,6 +29,7 @@
 #include <aws/core/utils/memory/stl/AWSStringStream.h>
 #include <aws/core/utils/HashingUtils.h>
 #include <aws/core/utils/Outcome.h>
+#include <aws/core/Globals.h>
 #include <aws/testing/mocks/http/MockHttpClient.h>
 
 using namespace Aws::Client;
@@ -408,3 +409,41 @@ TEST(AWSClientTest, TestHostHeaderWithStandardHttpPort)
     host = r4.GetHeaderValue(Aws::Http::HOST_HEADER);
     ASSERT_STREQ("example.amazonaws.com:80", host.c_str());
 }
+
+TEST(AWSClientTest, TestOverflowContainerLife)
+{
+    {
+        ClientConfiguration config;
+        MockAWSClient client(config);
+        auto container = Aws::GetEnumOverflowContainer();
+        ASSERT_NE(container, nullptr);
+    }
+
+    auto container = Aws::GetEnumOverflowContainer();
+    ASSERT_EQ(container, nullptr);
+}
+
+TEST(AWSClientTest, TestCopyingClientExtendsOverflowContainerLife)
+{
+    ClientConfiguration config;
+    auto client1 = Aws::MakeUnique<MockAWSClient>(ALLOCATION_TAG, config);
+    auto container = Aws::GetEnumOverflowContainer();
+    ASSERT_NE(container, nullptr);
+
+    // copy clien1 to client2
+    auto client2 = Aws::MakeUnique<MockAWSClient>(ALLOCATION_TAG, *client1);
+    client1 = nullptr;
+    container = Aws::GetEnumOverflowContainer();
+    ASSERT_NE(container, nullptr);
+
+    // move client2 to client3
+    auto client3 = Aws::MakeUnique<MockAWSClient>(ALLOCATION_TAG, std::move(*client2));
+    client2 = nullptr;
+    container = Aws::GetEnumOverflowContainer();
+    ASSERT_NE(container, nullptr);
+
+    // clear client3
+    client3 = nullptr;
+    container = Aws::GetEnumOverflowContainer();
+    ASSERT_EQ(container, nullptr);
+}

aws-cpp-sdk-core/include/aws/core/Globals.h

@@ -32,7 +32,7 @@ namespace Aws
     AWS_CORE_API Utils::EnumParseOverflowContainer* GetEnumOverflowContainer();
 
     /**
-     * Atomically set the underlying container to newValue, if it's current value is expectedValue.
+     * Sets the underlying container to newValue.
      */
-    AWS_CORE_API bool CheckAndSwapEnumOverflowContainer(Utils::EnumParseOverflowContainer* expectedValue, Utils::EnumParseOverflowContainer* newValue);
-}
+    void SetEnumOverflowContainer(Utils::EnumParseOverflowContainer* newValue);
+}

aws-cpp-sdk-core/include/aws/core/client/AWSClient.h

@@ -111,6 +111,11 @@ namespace Aws
                 const std::shared_ptr<Aws::Auth::AWSAuthSignerProvider>& signerProvider,
                 const std::shared_ptr<AWSErrorMarshaller>& errorMarshaller);
 
+            AWSClient(const AWSClient&);
+            AWSClient(AWSClient&&);
+            AWSClient& operator = (const AWSClient&);
+            AWSClient& operator = (AWSClient&&);
+
             virtual ~AWSClient();
 
             /**
@@ -244,7 +249,6 @@ namespace Aws
                                          const std::shared_ptr<Aws::IOStream>& body, bool needsContentMd5 = false) const;
             void AddCommonHeaders(Aws::Http::HttpRequest& httpRequest) const;
             void InitializeGlobalStatics();
-            void CleanupGlobalStatics();
             std::shared_ptr<Aws::Http::HttpRequest> ConvertToRequestForPresigning(const Aws::AmazonWebServiceRequest& request, Aws::Http::URI& uri,
                 Aws::Http::HttpMethod method, const Aws::Http::QueryStringParameterCollection& extraParams) const;
 

aws-cpp-sdk-core/include/aws/core/utils/EnumParseOverflowContainer.h

@@ -18,7 +18,7 @@
 #include <aws/core/utils/memory/stl/AWSMap.h>
 #include <aws/core/utils/memory/stl/AWSString.h>
 #include <aws/core/Core_EXPORTS.h>
-#include <mutex>
+#include <aws/core/utils/threading/ReaderWriterLock.h>
 
 namespace Aws
 {
@@ -36,9 +36,9 @@ namespace Aws
             void StoreOverflow(int hashCode, const Aws::String& value);
 
         private:
-            mutable std::mutex m_overflowLock;
+            mutable Aws::Utils::Threading::ReaderWriterLock m_overflowLock;
             Aws::Map<int, Aws::String> m_overflowMap;
-            Aws::String m_emptyString;
+            static const Aws::String m_emptyString;
         };
     }
 }

aws-cpp-sdk-core/source/Globals.cpp

@@ -18,15 +18,15 @@
 
 namespace Aws
 {
-    static std::atomic<Utils::EnumParseOverflowContainer*> g_enumOverflow(nullptr);
+    static Utils::EnumParseOverflowContainer* g_enumOverflow = nullptr;
 
     Utils::EnumParseOverflowContainer* GetEnumOverflowContainer()
     {
-        return g_enumOverflow.load();
+        return g_enumOverflow;
     }
 
-    bool CheckAndSwapEnumOverflowContainer(Utils::EnumParseOverflowContainer* expectedValue, Utils::EnumParseOverflowContainer* newValue)
+    void SetEnumOverflowContainer(Utils::EnumParseOverflowContainer* newValue)
     {
-        return g_enumOverflow.compare_exchange_strong(expectedValue, newValue);
+        g_enumOverflow = newValue;
     }
-}
+}

aws-cpp-sdk-core/source/client/AWSClient.cpp

@@ -74,40 +74,11 @@ static CoreErrors GuessBodylessErrorType(Aws::Http::HttpResponseCode responseCod
 
 void AWSClient::InitializeGlobalStatics()
 {
-    int currentRefCount = s_refCount.load();
-    if (!currentRefCount)
-    {      
-        int expectedRefCount = 0;    
-        Utils::EnumParseOverflowContainer* expectedPtrValue = nullptr;
-        Utils::EnumParseOverflowContainer* container = Aws::New<Utils::EnumParseOverflowContainer>(AWS_CLIENT_LOG_TAG);
-        if (!s_refCount.compare_exchange_strong(expectedRefCount, 1) ||
-             !Aws::CheckAndSwapEnumOverflowContainer(expectedPtrValue, container))
-        {
-            Aws::Delete(container);
-        }        
-    }
-    else
+    if (s_refCount++ == 0)
     {
-        ++s_refCount;
-    }
-}
-
-void AWSClient::CleanupGlobalStatics()
-{
-    int currentRefCount = s_refCount.load(); 
-    Utils::EnumParseOverflowContainer* expectedPtrValue = Aws::GetEnumOverflowContainer();
-
-    if (currentRefCount == 1)
-    {
-        if (s_refCount.compare_exchange_strong(currentRefCount, 0) &&
-            Aws::CheckAndSwapEnumOverflowContainer(expectedPtrValue, nullptr))
-        {
-            Aws::Delete(expectedPtrValue);           
-            return;
-        }        
+        Utils::EnumParseOverflowContainer* container = Aws::New<Utils::EnumParseOverflowContainer>(AWS_CLIENT_LOG_TAG);
+        Aws::SetEnumOverflowContainer(container);
     }
-
-    --s_refCount;
 }
 
 AWSClient::AWSClient(const Aws::Client::ClientConfiguration& configuration,
@@ -143,9 +114,77 @@ AWSClient::AWSClient(const Aws::Client::ClientConfiguration& configuration,
 }
 
 
+AWSClient::AWSClient(const AWSClient& other) :
+            m_httpClient(other.m_httpClient),
+            m_signerProvider(other.m_signerProvider),
+            m_errorMarshaller(other.m_errorMarshaller),
+            m_retryStrategy(other.m_retryStrategy),
+            m_writeRateLimiter(other.m_writeRateLimiter),
+            m_readRateLimiter(other.m_readRateLimiter),
+            m_userAgent(other.m_userAgent),
+            m_hash(other.m_hash),
+            m_enableClockSkewAdjustment(other.m_enableClockSkewAdjustment)
+{
+    s_refCount++;
+}
+
+AWSClient::AWSClient(AWSClient&& other) :
+            m_httpClient(std::move(other.m_httpClient)),
+            m_signerProvider(std::move(other.m_signerProvider)),
+            m_errorMarshaller(std::move(other.m_errorMarshaller)),
+            m_retryStrategy(std::move(other.m_retryStrategy)),
+            m_writeRateLimiter(std::move(other.m_writeRateLimiter)),
+            m_readRateLimiter(std::move(other.m_readRateLimiter)),
+            m_userAgent(std::move(other.m_userAgent)),
+            m_hash(std::move(other.m_hash)),
+            m_enableClockSkewAdjustment(other.m_enableClockSkewAdjustment)
+{
+    s_refCount++;
+}
+
+AWSClient& AWSClient::operator = (const AWSClient& other)
+{
+    if(this == &other)
+    {
+        return *this;
+    }
+
+    m_httpClient = other.m_httpClient;
+    m_signerProvider = other.m_signerProvider;
+    m_errorMarshaller = other.m_errorMarshaller;
+    m_retryStrategy = other.m_retryStrategy;
+    m_writeRateLimiter = other.m_writeRateLimiter;
+    m_readRateLimiter = other.m_readRateLimiter;
+    m_userAgent = other.m_userAgent;
+    m_hash = other.m_hash;
+    m_enableClockSkewAdjustment = other.m_enableClockSkewAdjustment;
+    s_refCount++;
+    return *this;
+}
+
+AWSClient& AWSClient::operator = (AWSClient&& other)
+{
+    m_httpClient = std::move(other.m_httpClient);
+    m_signerProvider = std::move(other.m_signerProvider);
+    m_errorMarshaller = std::move(other.m_errorMarshaller);
+    m_retryStrategy = std::move(other.m_retryStrategy);
+    m_writeRateLimiter = std::move(other.m_writeRateLimiter);
+    m_readRateLimiter = std::move(other.m_readRateLimiter);
+    m_userAgent = std::move(other.m_userAgent);
+    m_hash = std::move(other.m_hash);
+    m_enableClockSkewAdjustment = other.m_enableClockSkewAdjustment;
+    s_refCount++;
+    return *this;
+}
+
 AWSClient::~AWSClient()
 {
-    CleanupGlobalStatics();
+    if (--s_refCount == 0)
+    {
+        Utils::EnumParseOverflowContainer* expectedPtrValue = Aws::GetEnumOverflowContainer();
+        Aws::SetEnumOverflowContainer(nullptr);
+        Aws::Delete(expectedPtrValue);
+    }
 }
 
 void AWSClient::DisableRequestProcessing() 

aws-cpp-sdk-core/source/utils/EnumParseOverflowContainer.cpp

@@ -17,28 +17,29 @@
 #include <aws/core/utils/logging/LogMacros.h>
 
 using namespace Aws::Utils;
+using namespace Aws::Utils::Threading;
 
-static const char* LOG_TAG = "EnumParseOverflowContainer";
+static const char LOG_TAG[] = "EnumParseOverflowContainer";
+
+const Aws::String EnumParseOverflowContainer::m_emptyString("");
 
 const Aws::String& EnumParseOverflowContainer::RetrieveOverflow(int hashCode) const
 {
+    ReaderLockGuard guard(m_overflowLock);
+    auto foundIter = m_overflowMap.find(hashCode);
+    if (foundIter != m_overflowMap.end())
     {
-        std::lock_guard<std::mutex> locker(m_overflowLock);
-        auto foundIter = m_overflowMap.find(hashCode);
-        if (foundIter != m_overflowMap.end())
-        {
-            AWS_LOGSTREAM_DEBUG(LOG_TAG, "Found value " << foundIter->second << " for hash " << hashCode << " from enum overflow container.");
-            return foundIter->second;
-        } 
-    }  
+        AWS_LOGSTREAM_DEBUG(LOG_TAG, "Found value " << foundIter->second << " for hash " << hashCode << " from enum overflow container.");
+        return foundIter->second;
+    }
 
     AWS_LOGSTREAM_ERROR(LOG_TAG, "Could not find a previously stored overflow value for hash " << hashCode << ". This will likely break some requests.");
     return m_emptyString;
 }
 
 void EnumParseOverflowContainer::StoreOverflow(int hashCode, const Aws::String& value)
 {
+    WriterLockGuard guard(m_overflowLock);
     AWS_LOGSTREAM_WARN(LOG_TAG, "Encountered enum member " << value << " which is not modeled in your clients. You should update your clients when you get a chance.");
-    std::lock_guard<std::mutex> locker(m_overflowLock);
     m_overflowMap[hashCode] = value;
-}
+}