From cab995fd9a66083a7b8f591105f3a6d550571496 Mon Sep 17 00:00:00 2001
From: Dmitry Antipov <dmantipov@yandex.ru>
Date: Fri, 22 May 2020 15:06:57 +0300
Subject: rpc, gf_attach: add minimal proper synchronization

Implement minimal proper synchronization between gf_attach
and underlying RPC layer using convenient POSIX primitives.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
Fixes: #1260

Change-Id: Ib5130b586a8b65ed5cf5f9156c111b161570224b
---
 glusterfsd/src/gf_attach.c | 45 +++++++++++++++++++++++++++++++++++++--------
 rpc/rpc-lib/src/rpc-clnt.c |  3 +++
 rpc/rpc-lib/src/rpc-clnt.h |  1 +
 3 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/glusterfsd/src/gf_attach.c b/glusterfsd/src/gf_attach.c
index cb6c0f59b0..c54b4f9b51 100644
--- a/glusterfsd/src/gf_attach.c
+++ b/glusterfsd/src/gf_attach.c
@@ -19,9 +19,16 @@
 #include "xdr-generic.h"
 #include "glusterd1-xdr.h"
 
+/* In seconds */
+#define CONNECT_TIMEOUT 60
+#define REPLY_TIMEOUT 120
+
 int done = 0;
 int rpc_status;
 
+pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+
 struct rpc_clnt_procedure gf_attach_actors[GLUSTERD_BRICK_MAXVALUE] = {
     [GLUSTERD_BRICK_NULL] = {"NULL", NULL},
     [GLUSTERD_BRICK_OP] = {"BRICK_OP", NULL},
@@ -38,8 +45,12 @@ struct rpc_clnt_program gf_attach_prog = {
 int32_t
 my_callback(struct rpc_req *req, struct iovec *iov, int count, void *frame)
 {
+    pthread_mutex_lock(&mutex);
     rpc_status = req->rpc_status;
     done = 1;
+    /* Signal main thread which is the only waiter */
+    pthread_cond_signal(&cond);
+    pthread_mutex_unlock(&mutex);
     return 0;
 }
 
@@ -48,6 +59,7 @@ int
 send_brick_req(xlator_t *this, struct rpc_clnt *rpc, char *path, int op)
 {
     int ret = -1;
+    struct timespec ts;
     struct iobuf *iobuf = NULL;
     struct iobref *iobref = NULL;
     struct iovec iov = {
@@ -57,7 +69,6 @@ send_brick_req(xlator_t *this, struct rpc_clnt *rpc, char *path, int op)
     call_frame_t *frame = NULL;
     gd1_mgmt_brick_op_req brick_req;
     void *req = &brick_req;
-    int i;
 
     brick_req.op = op;
     brick_req.name = path;
@@ -87,12 +98,20 @@ send_brick_req(xlator_t *this, struct rpc_clnt *rpc, char *path, int op)
 
     iov.iov_len = ret;
 
-    for (i = 0; i < 60; ++i) {
-        if (rpc->conn.connected) {
-            break;
-        }
-        sleep(1);
+    /* Wait for connection */
+    clock_gettime(CLOCK_REALTIME, &ts);
+    ts.tv_sec += CONNECT_TIMEOUT;
+    pthread_mutex_lock(&rpc->conn.lock);
+    {
+        while (!rpc->conn.connected)
+            if (pthread_cond_timedwait(&rpc->conn.cond, &rpc->conn.lock, &ts) ==
+                ETIMEDOUT) {
+                fprintf(stderr, "timeout waiting for RPC connection\n");
+                pthread_mutex_unlock(&rpc->conn.lock);
+                return EXIT_FAILURE;
+            }
     }
+    pthread_mutex_unlock(&rpc->conn.lock);
 
     frame = create_frame(this, this->ctx->pool);
     if (!frame) {
@@ -104,9 +123,19 @@ send_brick_req(xlator_t *this, struct rpc_clnt *rpc, char *path, int op)
     ret = rpc_clnt_submit(rpc, &gf_attach_prog, op, my_callback, &iov, 1, NULL,
                           0, iobref, frame, NULL, 0, NULL, 0, NULL);
     if (!ret) {
-        for (i = 0; !done && (i < 120); ++i) {
-            sleep(1);
+        /* OK, wait for callback */
+        clock_gettime(CLOCK_REALTIME, &ts);
+        ts.tv_sec += REPLY_TIMEOUT;
+        pthread_mutex_lock(&mutex);
+        {
+            while (!done)
+                if (pthread_cond_timedwait(&cond, &mutex, &ts) == ETIMEDOUT) {
+                    fprintf(stderr, "timeout waiting for RPC reply\n");
+                    pthread_mutex_unlock(&mutex);
+                    return EXIT_FAILURE;
+                }
         }
+        pthread_mutex_unlock(&mutex);
     }
 
 out:
diff --git a/rpc/rpc-lib/src/rpc-clnt.c b/rpc/rpc-lib/src/rpc-clnt.c
index dac707664d..04b7750148 100644
--- a/rpc/rpc-lib/src/rpc-clnt.c
+++ b/rpc/rpc-lib/src/rpc-clnt.c
@@ -960,6 +960,7 @@ rpc_clnt_notify(rpc_transport_t *trans, void *mydata,
                 conn->config.remote_port = 0;
                 conn->connected = 1;
                 conn->disconnected = 0;
+                pthread_cond_broadcast(&conn->cond);
             }
             pthread_mutex_unlock(&conn->lock);
 
@@ -1005,6 +1006,7 @@ rpc_clnt_connection_init(struct rpc_clnt *clnt, glusterfs_ctx_t *ctx,
 
     conn = &clnt->conn;
     pthread_mutex_init(&clnt->conn.lock, NULL);
+    pthread_cond_init(&clnt->conn.cond, NULL);
 
     conn->name = gf_strdup(name);
     if (!conn->name) {
@@ -1830,6 +1832,7 @@ rpc_clnt_destroy(struct rpc_clnt *rpc)
     saved_frames_destroy(saved_frames);
     pthread_mutex_destroy(&rpc->lock);
     pthread_mutex_destroy(&rpc->conn.lock);
+    pthread_cond_destroy(&rpc->conn.cond);
 
     /* mem-pool should be destroyed, otherwise,
        it will cause huge memory leaks */
diff --git a/rpc/rpc-lib/src/rpc-clnt.h b/rpc/rpc-lib/src/rpc-clnt.h
index 0d0b115578..2945265200 100644
--- a/rpc/rpc-lib/src/rpc-clnt.h
+++ b/rpc/rpc-lib/src/rpc-clnt.h
@@ -127,6 +127,7 @@ struct rpc_clnt_config {
 
 struct rpc_clnt_connection {
     pthread_mutex_t lock;
+    pthread_cond_t cond;
     rpc_transport_t *trans;
     struct rpc_clnt_config config;
     gf_timer_t *reconnect;
-- 
cgit