librados: don't crash if we call connect twice

Fixes: #1034
Reported-by: Wido den Hollander <wido@widodh.nl>
Signed-off-by: Sage Weil <sage@newdream.net>

diff --git a/src/librados.cc b/src/librados.cc
index 7fa4741b46dc0d0bd3c1cdb0dd80ae1694bd9a95..74e6934d1b77fd6f991f484a8efde8c936fd7f3a 100644 (file)
--- a/src/librados.cc
+++ b/src/librados.cc
@@ -20,6 +20,7 @@
 #include <iostream>
 #include <string>
 #include <pthread.h>
+#include <errno.h>
 using namespace std;
 
 #include "common/config.h"
@@ -319,6 +320,12 @@ struct librados::ObjListCtx {
 
 class librados::RadosClient : public Dispatcher
 {
+  enum {
+    DISCONNECTED, 
+    CONNECTING,
+    CONNECTED,
+  } state;
+
   OSDMap osdmap;
   MonClient monclient;
   SimpleMessenger *messenger;
@@ -346,8 +353,9 @@ class librados::RadosClient : public Dispatcher
   SafeTimer timer;
 
 public:
-  RadosClient() : messenger(NULL), objecter(NULL),
-           lock("radosclient"), timer(lock), max_watch_cookie(0)
+  RadosClient() : state(DISCONNECTED),
+                 messenger(NULL), objecter(NULL),
+                 lock("radosclient"), timer(lock), max_watch_cookie(0)
   {
   }
 
@@ -622,30 +630,41 @@ public:
 int librados::RadosClient::
 connect()
 {
+  int err;
+  uint64_t nonce;
+
+  // already connected?
+  if (state == CONNECTING)
+    return -EINPROGRESS;
+  if (state == CONNECTED)
+    return -EISCONN;
+  state = CONNECTING;
+
   // get monmap
-  int ret = monclient.build_initial_monmap();
-  if (ret < 0)
-    return ret;
+  err = monclient.build_initial_monmap();
+  if (err < 0)
+    goto out;
 
+  err = -ENOMEM;
   messenger = new SimpleMessenger();
   if (!messenger)
-    return -ENOMEM;
+    goto out;
 
   dout(1) << "starting msgr at " << messenger->get_ms_addr() << dendl;
 
   messenger->register_entity(entity_name_t::CLIENT(-1));
   dout(1) << "starting objecter" << dendl;
 
+  err = -ENOMEM;
   objecter = new Objecter(messenger, &monclient, &osdmap, lock, timer);
   if (!objecter)
-    return -ENOMEM;
+    goto out;
   objecter->set_balanced_budget();
 
   monclient.set_messenger(messenger);
 
   messenger->add_dispatcher_head(this);
 
-  uint64_t nonce;
   rados_instance.inc();
   nonce = getpid() + (1000000 * (uint64_t)rados_instance.read());
 
@@ -657,11 +676,11 @@ connect()
   dout(1) << "calling monclient init" << dendl;
   monclient.init();
 
-  int err = monclient.authenticate(g_conf.client_mount_timeout);
+  err = monclient.authenticate(g_conf.client_mount_timeout);
   if (err) {
     dout(0) << g_conf.name << " authentication error " << strerror(-err) << dendl;
     shutdown();
-    return err;
+    goto out;
   }
   messenger->set_myname(entity_name_t::CLIENT(monclient.get_global_id()));
 
@@ -677,11 +696,16 @@ connect()
     dout(1) << "waiting for osdmap" << dendl;
     cond.Wait(lock);
   }
+  state = CONNECTED;
   lock.Unlock();
 
   dout(1) << "init done" << dendl;
+  err = 0;
 
-  return 0;
+ out:
+  if (err)
+    state = DISCONNECTED;
+  return err;
 }
 
 void librados::RadosClient::
@@ -692,6 +716,7 @@ shutdown()
   if (objecter)
     objecter->shutdown();
   timer.shutdown();
+  state = DISCONNECTED;
   lock.Unlock();
   if (messenger) {
     messenger->shutdown();

diff --git a/src/testrados.c b/src/testrados.c
index 7991f57e684ae36c60cf2bc15071d15e12702c3d..38994b7782a7301c6da1e47b9e122b800622f2c6 100644 (file)
--- a/src/testrados.c
+++ b/src/testrados.c
@@ -65,6 +65,10 @@ int main(int argc, const char **argv)
     printf("error connecting\n");
     exit(1);
   }
+  if (rados_connect(cl) == 0) {
+    printf("second connect attempt didn't return an error\n");
+    exit(1);
+  }
 
   /* create an io_ctx */
   r = rados_pool_create(cl, "foo");