#133: librados::Rados is now created on heap (init) destroyed (deinit…

…()) is called.
ceph-dovecot · Apr 19, 2018 · e08b6c2 · e08b6c2
1 parent df86f85
commit e08b6c2
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 37 deletions.
diff --git a/src/librmb/rados-cluster-impl.cpp b/src/librmb/rados-cluster-impl.cpp
@@ -34,7 +34,7 @@ const char *RadosClusterImpl::RADOS_OSD_OP_TIMEOUT_DEFAULT = "10";
 
 // Note: Using Dictionary und RadosStorage with different ceph cluster / user is currently
 //       not supported.
-librados::Rados RadosClusterImpl::cluster;
+librados::Rados *RadosClusterImpl::cluster = 0;
 int RadosClusterImpl::cluster_ref_count = 0;
 bool RadosClusterImpl::connected = false;
 
@@ -47,7 +47,8 @@ RadosClusterImpl::~RadosClusterImpl() {}
 int RadosClusterImpl::init() {
   int ret = 0;
   if (RadosClusterImpl::cluster_ref_count == 0) {
-    ret = RadosClusterImpl::cluster.init(nullptr);
+    RadosClusterImpl::cluster = new librados::Rados();
+    ret = RadosClusterImpl::cluster->init(nullptr);
     if (ret == 0) {
       ret = initialize();
     }
@@ -60,7 +61,8 @@ int RadosClusterImpl::init() {
 int RadosClusterImpl::init(const std::string &clustername, const std::string &rados_username) {
   int ret = 0;
   if (RadosClusterImpl::cluster_ref_count == 0) {
-    ret = RadosClusterImpl::cluster.init2(rados_username.c_str(), clustername.c_str(), 0);
+    RadosClusterImpl::cluster = new librados::Rados();
+    ret = RadosClusterImpl::cluster->init2(rados_username.c_str(), clustername.c_str(), 0);
     if (ret == 0) {
       ret = initialize();
     }
@@ -75,25 +77,25 @@ int RadosClusterImpl::initialize() {
   int ret = 0;
 
   if (ret == 0) {
-    ret = RadosClusterImpl::cluster.conf_parse_env(nullptr);
+    ret = RadosClusterImpl::cluster->conf_parse_env(nullptr);
   }
 
   if (ret == 0) {
-    ret = RadosClusterImpl::cluster.conf_read_file(nullptr);
+    ret = RadosClusterImpl::cluster->conf_read_file(nullptr);
   }
   // check if ceph configuration has connection timeout set, else set defaults to avoid
   // waiting forever
   std::string cfg_value;
   if (get_config_option(CLIENT_MOUNT_TIMEOUT, &cfg_value) < 0) {
-    RadosClusterImpl::cluster.conf_set(CLIENT_MOUNT_TIMEOUT, CLIENT_MOUNT_TIMEOUT_DEFAULT);
+    RadosClusterImpl::cluster->conf_set(CLIENT_MOUNT_TIMEOUT, CLIENT_MOUNT_TIMEOUT_DEFAULT);
   }
   ret = get_config_option(RADOS_MON_OP_TIMEOUT, &cfg_value);
   if (ret < 0 || cfg_value.compare("0") == 0) {
-    RadosClusterImpl::cluster.conf_set(RADOS_MON_OP_TIMEOUT, RADOS_MON_OP_TIMEOUT_DEFAULT);
+    RadosClusterImpl::cluster->conf_set(RADOS_MON_OP_TIMEOUT, RADOS_MON_OP_TIMEOUT_DEFAULT);
   }
   ret = get_config_option(RADOS_OSD_OP_TIMEOUT, &cfg_value);
   if (ret < 0 || cfg_value.compare("0") == 0) {
-    RadosClusterImpl::cluster.conf_set(RADOS_OSD_OP_TIMEOUT, RADOS_OSD_OP_TIMEOUT_DEFAULT);
+    RadosClusterImpl::cluster->conf_set(RADOS_OSD_OP_TIMEOUT, RADOS_OSD_OP_TIMEOUT_DEFAULT);
   }
 
   return ret;
@@ -104,7 +106,7 @@ bool RadosClusterImpl::is_connected() { return RadosClusterImpl::connected; }
 int RadosClusterImpl::connect() {
   int ret = 0;
   if (RadosClusterImpl::cluster_ref_count > 0 && !RadosClusterImpl::connected) {
-    ret = RadosClusterImpl::cluster.connect();
+    ret = RadosClusterImpl::cluster->connect();
     RadosClusterImpl::connected = ret == 0;
   }
   return ret;
@@ -114,8 +116,9 @@ void RadosClusterImpl::deinit() {
   if (RadosClusterImpl::cluster_ref_count > 0) {
     if (--RadosClusterImpl::cluster_ref_count == 0) {
       if (RadosClusterImpl::connected) {
-        RadosClusterImpl::cluster.shutdown();
+        RadosClusterImpl::cluster->shutdown();
         RadosClusterImpl::connected = false;
+        delete RadosClusterImpl::cluster;
       }
     }
   }
@@ -127,7 +130,7 @@ int RadosClusterImpl::pool_create(const string &pool) {
   int ret = connect();
   if (ret == 0) {
     list<pair<int64_t, string>> pool_list;
-    ret = RadosClusterImpl::cluster.pool_list2(pool_list);
+    ret = RadosClusterImpl::cluster->pool_list2(pool_list);
 
     if (ret == 0) {
       bool pool_found = false;
@@ -140,7 +143,7 @@ int RadosClusterImpl::pool_create(const string &pool) {
       }
 
       if (pool_found != true) {
-        ret = RadosClusterImpl::cluster.pool_create(pool.c_str());
+        ret = RadosClusterImpl::cluster->pool_create(pool.c_str());
         pool_found = ret == 0;
       }
     }
@@ -167,13 +170,13 @@ int RadosClusterImpl::io_ctx_create(const string &pool, librados::IoCtx *io_ctx)
     }
 
     if (ret == 0) {
-      ret = RadosClusterImpl::cluster.ioctx_create(pool.c_str(), *io_ctx);
+      ret = RadosClusterImpl::cluster->ioctx_create(pool.c_str(), *io_ctx);
     }
   }
 
   return ret;
 }
 
 int RadosClusterImpl::get_config_option(const char *option, string *value) {
-  return RadosClusterImpl::cluster.conf_get(option, *value);
+  return RadosClusterImpl::cluster->conf_get(option, *value);
 }
diff --git a/src/librmb/rados-cluster-impl.h b/src/librmb/rados-cluster-impl.h
@@ -37,13 +37,13 @@ class RadosClusterImpl : public RadosCluster {
   int dictionary_create(const std::string &pool, const std::string &username, const std::string &oid,
                         RadosDictionary **dictionary);
   bool is_connected();
-  librados::Rados &get_cluster() { return cluster; }
+  librados::Rados &get_cluster() { return *cluster; }
 
  private:
   int initialize();
 
  private:
-  static librados::Rados cluster;
+  static librados::Rados *cluster;
   static int cluster_ref_count;
   static bool connected;
 

diff --git a/src/storage-rbox/rbox-storage.cpp b/src/storage-rbox/rbox-storage.cpp
@@ -56,22 +56,6 @@ class RboxGuidGenerator : public librmb::RadosGuidGenerator {
 extern struct mailbox rbox_mailbox;
 extern struct mailbox_vfuncs rbox_mailbox_vfuncs;
 
-// if process is forked, child process
-// also tries to call ~librados::Rados() which leads
-// parent process wait forever at wait()
-// In case it's a child process it's safe to
-// _exit(0) here without calling additional
-// exit handlers.
-static int pid;
-static void rbox_storage_exit_handler(void) {
-  int current_pid = getpid();
-  if (current_pid != pid) {
-    // fork detected. this is the child process,
-    // exit with _exit to avoid additionaly exit handlers.
-    _exit(EXIT_SUCCESS);
-  }
-}
-
 struct mail_storage *rbox_storage_alloc(void) {
   FUNC_START();
   struct rbox_storage *storage;
@@ -86,11 +70,6 @@ struct mail_storage *rbox_storage_alloc(void) {
   storage->ns_mgr = new librmb::RadosNamespaceManager(storage->config);
   storage->ms = new librmb::RadosMetadataStorageImpl();
 
-  // register exit handler and save parent
-  // pid in case this process gets forked.
-  pid = getpid();
-  atexit(rbox_storage_exit_handler);
-
   FUNC_END();
   return &storage->storage;
 }