From c8f0d11918cc7d3577d624f2d757dc975c8bfad7 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Tue, 19 May 2015 15:01:08 +0530 Subject: rpc: fix possible deadlock left behind in d448fd1 See http://review.gluster.org/9613 for more details. BUG: 1233019 Change-Id: I05ac0267b8c6f4e9b354acbbdf5469835455fb10 Signed-off-by: Krishnan Parthasarathi Reviewed-on: http://review.gluster.org/10821 Reviewed-by: Raghavendra G Tested-by: Raghavendra G (cherry picked from commit e902df70f8157db4db503b7ec3c2635b08b3dcb2) Reviewed-on: http://review.gluster.org/11303 Tested-by: Gluster Build System --- rpc/rpc-lib/src/rpc-clnt-ping.c | 12 ++++++------ rpc/rpc-lib/src/rpc-clnt-ping.h | 2 ++ rpc/rpc-lib/src/rpc-clnt.c | 30 +++++++++++++----------------- 3 files changed, 21 insertions(+), 23 deletions(-) (limited to 'rpc/rpc-lib') diff --git a/rpc/rpc-lib/src/rpc-clnt-ping.c b/rpc/rpc-lib/src/rpc-clnt-ping.c index 77f3e32001c..3f794dbdd6a 100644 --- a/rpc/rpc-lib/src/rpc-clnt-ping.c +++ b/rpc/rpc-lib/src/rpc-clnt-ping.c @@ -75,8 +75,8 @@ __rpc_clnt_rearm_ping_timer (struct rpc_clnt *rpc, gf_timer_cbk_t cbk) } /* Must be called under conn->lock */ -static int -__rpc_clnt_remove_ping_timer (struct rpc_clnt *rpc) +int +rpc_clnt_remove_ping_timer_locked (struct rpc_clnt *rpc) { rpc_clnt_connection_t *conn = &rpc->conn; gf_timer_t *timer = NULL; @@ -126,7 +126,7 @@ rpc_clnt_ping_timer_expired (void *rpc_ptr) pthread_mutex_lock (&conn->lock); { - unref = __rpc_clnt_remove_ping_timer (rpc); + unref = rpc_clnt_remove_ping_timer_locked (rpc); gettimeofday (¤t, NULL); if (((current.tv_sec - conn->last_received.tv_sec) < @@ -197,7 +197,7 @@ rpc_clnt_ping_cbk (struct rpc_req *req, struct iovec *iov, int count, pthread_mutex_lock (&conn->lock); { if (req->rpc_status == -1) { - unref = __rpc_clnt_remove_ping_timer (rpc); + unref = rpc_clnt_remove_ping_timer_locked (rpc); if (unref) { gf_log (this->name, GF_LOG_WARNING, "socket or ib related error"); @@ -212,7 +212,7 @@ rpc_clnt_ping_cbk (struct rpc_req *req, struct iovec *iov, int count, goto unlock; } - unref = __rpc_clnt_remove_ping_timer (rpc); + unref = rpc_clnt_remove_ping_timer_locked (rpc); if (__rpc_clnt_rearm_ping_timer (rpc, rpc_clnt_start_ping) == -1) { gf_log (this->name, GF_LOG_WARNING, @@ -284,7 +284,7 @@ rpc_clnt_start_ping (void *rpc_ptr) pthread_mutex_lock (&conn->lock); { - unref = __rpc_clnt_remove_ping_timer (rpc); + unref = rpc_clnt_remove_ping_timer_locked (rpc); if (conn->saved_frames) { GF_ASSERT (conn->saved_frames->count >= 0); diff --git a/rpc/rpc-lib/src/rpc-clnt-ping.h b/rpc/rpc-lib/src/rpc-clnt-ping.h index d7cd1d965e5..a20997fa1af 100644 --- a/rpc/rpc-lib/src/rpc-clnt-ping.h +++ b/rpc/rpc-lib/src/rpc-clnt-ping.h @@ -17,3 +17,5 @@ #define RPC_DEFAULT_PING_TIMEOUT 30 void rpc_clnt_check_and_start_ping (struct rpc_clnt *rpc_ptr); +int +rpc_clnt_remove_ping_timer_locked (struct rpc_clnt *rpc); diff --git a/rpc/rpc-lib/src/rpc-clnt.c b/rpc/rpc-lib/src/rpc-clnt.c index 1d878663611..790436a4f2d 100644 --- a/rpc/rpc-lib/src/rpc-clnt.c +++ b/rpc/rpc-lib/src/rpc-clnt.c @@ -500,6 +500,7 @@ rpc_clnt_connection_cleanup (rpc_clnt_connection_t *conn) { struct saved_frames *saved_frames = NULL; struct rpc_clnt *clnt = NULL; + int unref = 0; if (!conn) { goto out; @@ -521,12 +522,7 @@ rpc_clnt_connection_cleanup (rpc_clnt_connection_t *conn) conn->connected = 0; - if (conn->ping_timer) { - gf_timer_call_cancel (clnt->ctx, conn->ping_timer); - conn->ping_timer = NULL; - conn->ping_started = 0; - rpc_clnt_unref (clnt); - } + unref = rpc_clnt_remove_ping_timer_locked (clnt); /*reset rpc msgs stats*/ conn->pingcnt = 0; conn->msgcnt = 0; @@ -534,6 +530,8 @@ rpc_clnt_connection_cleanup (rpc_clnt_connection_t *conn) pthread_mutex_unlock (&conn->lock); saved_frames_destroy (saved_frames); + if (unref) + rpc_clnt_unref (clnt); out: return 0; @@ -1731,6 +1729,7 @@ rpc_clnt_disable (struct rpc_clnt *rpc) { rpc_clnt_connection_t *conn = NULL; rpc_transport_t *trans = NULL; + int unref = 0; if (!rpc) { goto out; @@ -1753,12 +1752,7 @@ rpc_clnt_disable (struct rpc_clnt *rpc) } conn->connected = 0; - if (conn->ping_timer) { - gf_timer_call_cancel (rpc->ctx, conn->ping_timer); - conn->ping_timer = NULL; - conn->ping_started = 0; - rpc_clnt_unref (rpc); - } + unref = rpc_clnt_remove_ping_timer_locked (rpc); trans = conn->trans; conn->trans = NULL; @@ -1769,6 +1763,9 @@ rpc_clnt_disable (struct rpc_clnt *rpc) rpc_transport_disconnect (trans); } + if (unref) + rpc_clnt_unref (rpc); + out: return; } @@ -1778,6 +1775,7 @@ rpc_clnt_disconnect (struct rpc_clnt *rpc) { rpc_clnt_connection_t *conn = NULL; rpc_transport_t *trans = NULL; + int unref = 0; if (!rpc) goto out; @@ -1798,11 +1796,7 @@ rpc_clnt_disconnect (struct rpc_clnt *rpc) } conn->connected = 0; - if (conn->ping_timer) { - gf_timer_call_cancel (rpc->ctx, conn->ping_timer); - conn->ping_timer = NULL; - conn->ping_started = 0; - } + unref = rpc_clnt_remove_ping_timer_locked (rpc); trans = conn->trans; } pthread_mutex_unlock (&conn->lock); @@ -1810,6 +1804,8 @@ rpc_clnt_disconnect (struct rpc_clnt *rpc) if (trans) { rpc_transport_disconnect (trans); } + if (unref) + rpc_clnt_unref (rpc); out: return; -- cgit