Skip to content

Commit

Permalink
cli: fix data race when handling connection status
Browse files Browse the repository at this point in the history
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 <null> <null> (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 <null> <null> (libtsan.so.0+0x2d33f)

  Location is global 'connected' of size 4 at 0x00000047dfa0 (gluster+0x00000047dfa0)

Change-Id: Ie85a8a80a2c5b82252c0c1d45e68ebe9938da2eb
Signed-off-by: Dmitry Antipov <[email protected]>
Fixes: #1311
  • Loading branch information
dmantipov authored and amarts committed Jun 18, 2020
1 parent 3510916 commit 2ef7518
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 11 deletions.
21 changes: 17 additions & 4 deletions cli/src/cli-cmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions cli/src/cli-rpc-ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 2 additions & 3 deletions cli/src/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@

#include "xdr-generic.h"

extern int connected;
/* using argp for command line parsing */

const char *argp_program_version =
Expand Down Expand Up @@ -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(
Expand Down
7 changes: 5 additions & 2 deletions cli/src/cli.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 2ef7518

Please sign in to comment.