msgr: use common helper for reader/writer thread stop and reap queueing; fix locking

We weren't holding lock during ::close(sd)

diff --git a/src/msg/SimpleMessenger.cc b/src/msg/SimpleMessenger.cc
index d1ffb8dd83aef50a5780f1ca6c3877f05cc36e27..a0bbae6c45fd8638bdf943a60c5d1691d30b5c0f 100644 (file)
--- a/src/msg/SimpleMessenger.cc
+++ b/src/msg/SimpleMessenger.cc
@@ -1380,43 +1380,11 @@ void SimpleMessenger::Pipe::reader()
 
  
   // reap?
-  bool reap = false;
   reader_running = false;
-  if (!writer_running)
-    reap = true;
-
-  lock.Unlock();
-
-  if (reap) {
-    dout(10) << "reader queueing for reap" << dendl;
-    if (sd >= 0) {
-      ::close(sd);
-      sd = -1;
-      closed_socket();
-    }
-    rank->lock.Lock();
-    {
-      rank->pipe_reap_queue.push_back(this);
-      rank->wait_cond.Signal();
-    }
-    rank->lock.Unlock();
-  }
-
+  unlock_maybe_reap();
   dout(10) << "reader done" << dendl;
 }
 
-/*
-class FakeSocketError : public Context {
-  int sd;
-public:
-  FakeSocketError(int s) : sd(s) {}
-  void finish(int r) {
-    cout << "faking socket error on " << sd << std::endl;
-    ::close(sd);
-  }
-};
-*/
-
 /* write msgs to socket.
  * also, client.
  */
@@ -1519,28 +1487,34 @@ void SimpleMessenger::Pipe::writer()
   dout(20) << "writer finishing" << dendl;
 
   // reap?
-  bool reap = false;
   writer_running = false;
-  if (!reader_running) reap = true;
+  unlock_maybe_reap();
+  dout(10) << "writer done" << dendl;
+}
 
-  lock.Unlock();
-  
-  if (reap) {
-    dout(10) << "writer queueing for reap" << dendl;
+void SimpleMessenger::Pipe::unlock_maybe_reap()
+{
+  if (!reader_running && !writer_running) {
+    // close
     if (sd >= 0) {
       ::close(sd);
       sd = -1;
       closed_socket();
     }
+
+    lock.Unlock();
+
+    // queue for reap
+    dout(10) << "unlock_maybe_reap queueing for reap" << dendl;
     rank->lock.Lock();
     {
       rank->pipe_reap_queue.push_back(this);
       rank->wait_cond.Signal();
     }
     rank->lock.Unlock();
+  } else {
+    lock.Unlock();
   }
-
-  dout(10) << "writer done" << dendl;
 }
 
 

diff --git a/src/msg/SimpleMessenger.h b/src/msg/SimpleMessenger.h
index 9ad6d87a23de12018d17aef19fecf19688dc5852..e27ec75a28720b8fcc510c458ab8379df66029d8 100644 (file)
--- a/src/msg/SimpleMessenger.h
+++ b/src/msg/SimpleMessenger.h
@@ -121,6 +121,7 @@ private:
     int connect();  // client handshake
     void reader();
     void writer();
+    void unlock_maybe_reap();
 
     Message *read_message();
     int write_message(Message *m);