From 16fad9c2cdff9f890ec771975fe284027536b74b Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Fri, 9 Dec 2016 10:12:56 +0200
Subject: [PATCH] Cache: Make storage_rocksdb exception safe

Now RocksDBStorage is exception safe in its own right, so exception
guards are only needed at the C/C++ boundary.
---
 .../storage/storage_rocksdb/rocksdbstorage.cc  | 18 ++++++++++--------
 .../storage/storage_rocksdb/rocksdbstorage.hh  |  6 ++++--
 .../storage/storage_rocksdb/storage_rocksdb.cc | 12 +++++++-----
 3 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/server/modules/filter/cache/storage/storage_rocksdb/rocksdbstorage.cc b/server/modules/filter/cache/storage/storage_rocksdb/rocksdbstorage.cc
index 7972c4ce4..b1b9aab4f 100644
--- a/server/modules/filter/cache/storage/storage_rocksdb/rocksdbstorage.cc
+++ b/server/modules/filter/cache/storage/storage_rocksdb/rocksdbstorage.cc
@@ -223,7 +223,9 @@ bool RocksDBStorage::Initialize()
 }
 
 //static
-RocksDBStorage* RocksDBStorage::Create(const char* zName, uint32_t ttl, int argc, char* argv[])
+unique_ptr<RocksDBStorage> RocksDBStorage::Create(const char* zName,
+                                                  uint32_t ttl,
+                                                  int argc, char* argv[])
 {
     ss_dassert(zName);
 
@@ -278,12 +280,12 @@ RocksDBStorage* RocksDBStorage::Create(const char* zName, uint32_t ttl, int argc
 }
 
 // static
-RocksDBStorage* RocksDBStorage::Create(const string& storageDirectory,
-                                       const char* zName,
-                                       uint32_t ttl,
-                                       bool collectStatistics)
+unique_ptr<RocksDBStorage> RocksDBStorage::Create(const string& storageDirectory,
+                                                  const char* zName,
+                                                  uint32_t ttl,
+                                                  bool collectStatistics)
 {
-    RocksDBStorage* pStorage = nullptr;
+    unique_ptr<RocksDBStorage> sStorage;
 
     if (mkdir(storageDirectory.c_str(), S_IRWXU) == 0)
     {
@@ -341,7 +343,7 @@ RocksDBStorage* RocksDBStorage::Create(const string& storageDirectory,
 
                 unique_ptr<rocksdb::DBWithTTL> sDb(pDb);
 
-                pStorage = new RocksDBStorage(sDb, zName, path, ttl);
+                sStorage = unique_ptr<RocksDBStorage>(new RocksDBStorage(sDb, zName, path, ttl));
             }
             else
             {
@@ -356,7 +358,7 @@ RocksDBStorage* RocksDBStorage::Create(const string& storageDirectory,
         }
     }
 
-    return pStorage;
+    return sStorage;
 }
 
 cache_result_t RocksDBStorage::getInfo(uint32_t what, json_t** ppInfo) const
diff --git a/server/modules/filter/cache/storage/storage_rocksdb/rocksdbstorage.hh b/server/modules/filter/cache/storage/storage_rocksdb/rocksdbstorage.hh
index 618a89d83..1d416c084 100644
--- a/server/modules/filter/cache/storage/storage_rocksdb/rocksdbstorage.hh
+++ b/server/modules/filter/cache/storage/storage_rocksdb/rocksdbstorage.hh
@@ -21,9 +21,11 @@
 class RocksDBStorage
 {
 public:
+    typedef std::unique_ptr<RocksDBStorage> SRocksDBStorage;
+
     static bool Initialize();
 
-    static RocksDBStorage* Create(const char* zName, uint32_t ttl, int argc, char* argv[]);
+    static SRocksDBStorage Create(const char* zName, uint32_t ttl, int argc, char* argv[]);
     ~RocksDBStorage();
 
     cache_result_t getInfo(uint32_t flags, json_t** ppInfo) const;
@@ -41,7 +43,7 @@ private:
     RocksDBStorage(const RocksDBStorage&) = delete;
     RocksDBStorage& operator = (const RocksDBStorage&) = delete;
 
-    static RocksDBStorage* Create(const std::string& storageDirectory,
+    static SRocksDBStorage Create(const std::string& storageDirectory,
                                   const char* zName,
                                   uint32_t ttl,
                                   bool collectStatistics);
diff --git a/server/modules/filter/cache/storage/storage_rocksdb/storage_rocksdb.cc b/server/modules/filter/cache/storage/storage_rocksdb/storage_rocksdb.cc
index ef0756058..1b8f0040a 100644
--- a/server/modules/filter/cache/storage/storage_rocksdb/storage_rocksdb.cc
+++ b/server/modules/filter/cache/storage/storage_rocksdb/storage_rocksdb.cc
@@ -17,6 +17,8 @@
 #include "../../cache_storage_api.h"
 #include "rocksdbstorage.hh"
 
+using std::unique_ptr;
+
 namespace
 {
 
@@ -48,21 +50,21 @@ CACHE_STORAGE* createInstance(cache_thread_model_t, // Ignored, RocksDB always M
                     "does not enforce such a limit.", (unsigned long)maxSize);
     }
 
-    RocksDBStorage* pStorage = NULL;
+    unique_ptr<RocksDBStorage> sStorage;
 
-    MXS_EXCEPTION_GUARD(pStorage = RocksDBStorage::Create(zName, ttl, argc, argv));
+    MXS_EXCEPTION_GUARD(sStorage = RocksDBStorage::Create(zName, ttl, argc, argv));
 
-    if (pStorage)
+    if (sStorage)
     {
         MXS_NOTICE("Storage module created.");
     }
 
-    return reinterpret_cast<CACHE_STORAGE*>(pStorage);
+    return reinterpret_cast<CACHE_STORAGE*>(sStorage.release());
 }
 
 void freeInstance(CACHE_STORAGE* pInstance)
 {
-    delete reinterpret_cast<RocksDBStorage*>(pInstance);
+    MXS_EXCEPTION_GUARD(delete reinterpret_cast<RocksDBStorage*>(pInstance));
 }
 
 cache_result_t getInfo(CACHE_STORAGE* pStorage,