From 2ef75183ab3faa68b79d3d38318d4120cfc657d1 Mon Sep 17 00:00:00 2001 From: Dmitry Antipov Date: Tue, 16 Jun 2020 14:45:03 +0300 Subject: [PATCH] cli: fix data race when handling connection status Found with GCC ThreadSanitizer: WARNING: ThreadSanitizer: data race (pid=287943) Write of size 4 at 0x00000047dfa0 by thread T4: #0 cli_rpc_notify /path/to/glusterfs/cli/src/cli.c:313 (gluster+0x40a6df) #1 rpc_clnt_handle_disconnect /path/to/glusterfs/rpc/rpc-lib/src/rpc-clnt.c:821 (libgfrpc.so.0+0x13f04) #2 rpc_clnt_notify /path/to/glusterfs/rpc/rpc-lib/src/rpc-clnt.c:882 (libgfrpc.so.0+0x13f04) #3 rpc_transport_notify /path/to/glusterfs/rpc/rpc-lib/src/rpc-transport.c:520 (libgfrpc.so.0+0xf070) #4 socket_event_poll_err /path/to/glusterfs/rpc/rpc-transport/socket/src/socket.c:1364 (socket.so+0x812c) #5 socket_event_handler /path/to/glusterfs/rpc/rpc-transport/socket/src/socket.c:2958 (socket.so+0xc453) #6 socket_event_handler /path/to/glusterfs/rpc/rpc-transport/socket/src/socket.c:2854 (socket.so+0xc453) #7 event_dispatch_epoll_handler /path/to/glusterfs/libglusterfs/src/event-epoll.c:640 (libglusterfs.so.0+0xcaf23) #8 event_dispatch_epoll_worker /path/to/glusterfs/libglusterfs/src/event-epoll.c:751 (libglusterfs.so.0+0xcaf23) #9 (libtsan.so.0+0x2d33f) Previous read of size 4 at 0x00000047dfa0 by thread T3 (mutexes: write M3587): #0 cli_cmd_await_connected /path/to/glusterfs/cli/src/cli-cmd.c:321 (gluster+0x40ca37) #1 cli_cmd_process /path/to/glusterfs/cli/src/cli-cmd.c:123 (gluster+0x40cc74) #2 cli_batch /path/to/glusterfs/cli/src/input.c:29 (gluster+0x40c2b9) #3 (libtsan.so.0+0x2d33f) Location is global 'connected' of size 4 at 0x00000047dfa0 (gluster+0x00000047dfa0) Change-Id: Ie85a8a80a2c5b82252c0c1d45e68ebe9938da2eb Signed-off-by: Dmitry Antipov Fixes: #1311 --- cli/src/cli-cmd.c | 21 +++++++++++++++++---- cli/src/cli-rpc-ops.c | 3 +-- cli/src/cli.c | 5 ++--- cli/src/cli.h | 7 +++++-- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/cli/src/cli-cmd.c b/cli/src/cli-cmd.c index af009757e9..2d458b16a5 100644 --- a/cli/src/cli-cmd.c +++ b/cli/src/cli-cmd.c @@ -28,7 +28,7 @@ static pthread_cond_t conn = PTHREAD_COND_INITIALIZER; static pthread_mutex_t conn_mutex = PTHREAD_MUTEX_INITIALIZER; int cli_op_ret = 0; -int connected = 0; +static gf_boolean_t connected = _gf_false; static unsigned cli_cmd_needs_connection(struct cli_cmd_word *word) @@ -328,19 +328,32 @@ cli_cmd_await_connected(unsigned conn_timo) } int32_t -cli_cmd_broadcast_connected() +cli_cmd_broadcast_connected(gf_boolean_t status) { pthread_mutex_lock(&conn_mutex); { - connected = 1; + connected = status; pthread_cond_broadcast(&conn); } - pthread_mutex_unlock(&conn_mutex); return 0; } +gf_boolean_t +cli_cmd_connected(void) +{ + gf_boolean_t status; + + pthread_mutex_lock(&conn_mutex); + { + status = connected; + } + pthread_mutex_unlock(&conn_mutex); + + return status; +} + int cli_cmd_submit(struct rpc_clnt *rpc, void *req, call_frame_t *frame, rpc_clnt_prog_t *prog, int procnum, struct iobref *iobref, diff --git a/cli/src/cli-rpc-ops.c b/cli/src/cli-rpc-ops.c index 7a97798ae3..e9cb81948e 100644 --- a/cli/src/cli-rpc-ops.c +++ b/cli/src/cli-rpc-ops.c @@ -50,7 +50,6 @@ enum gf_task_types { GF_TASK_TYPE_REBALANCE, GF_TASK_TYPE_REMOVE_BRICK }; extern struct rpc_clnt *global_quotad_rpc; rpc_clnt_prog_t cli_quotad_clnt; extern rpc_clnt_prog_t *cli_rpc_prog; -extern int connected; static int32_t gf_cli_remove_brick(call_frame_t *frame, xlator_t *this, void *data); @@ -3406,7 +3405,7 @@ gf_cli_quota_list(cli_local_t *local, char *volname, dict_t *dict, char *default_sl, int count, int op_ret, int op_errno, char *op_errstr) { - if (!connected) + if (!cli_cmd_connected()) goto out; if (count > 0) { diff --git a/cli/src/cli.c b/cli/src/cli.c index b42d29df9f..80b82bf77d 100644 --- a/cli/src/cli.c +++ b/cli/src/cli.c @@ -55,7 +55,6 @@ #include "xdr-generic.h" -extern int connected; /* using argp for command line parsing */ const char *argp_program_version = @@ -303,14 +302,14 @@ cli_rpc_notify(struct rpc_clnt *rpc, void *mydata, rpc_clnt_event_t event, switch (event) { case RPC_CLNT_CONNECT: { - cli_cmd_broadcast_connected(); + cli_cmd_broadcast_connected(_gf_true); gf_log(this->name, GF_LOG_TRACE, "got RPC_CLNT_CONNECT"); break; } case RPC_CLNT_DISCONNECT: { + cli_cmd_broadcast_connected(_gf_false); gf_log(this->name, GF_LOG_TRACE, "got RPC_CLNT_DISCONNECT"); - connected = 0; if (!global_state->prompt && global_state->await_connected) { ret = 1; cli_out( diff --git a/cli/src/cli.h b/cli/src/cli.h index cd4db3dac2..880bd00e55 100644 --- a/cli/src/cli.h +++ b/cli/src/cli.h @@ -329,11 +329,14 @@ cli_local_get(); void cli_local_wipe(cli_local_t *local); +gf_boolean_t +cli_cmd_connected(); + int32_t -cli_cmd_await_connected(); +cli_cmd_await_connected(unsigned timeout); int32_t -cli_cmd_broadcast_connected(); +cli_cmd_broadcast_connected(gf_boolean_t status); int cli_rpc_notify(struct rpc_clnt *rpc, void *mydata, rpc_clnt_event_t event,