Skip to content

Commit 2b551df

Browse files
pushkarscriptsgitster
authored andcommitted
transport-helper: fix TSAN race in transfer_debug()
Currently, transfer_debug() lazily initializes a static variable based on GIT_TRANSLOOP_DEBUG. Since the function may be called from multiple worker threads, this initialization is racy and is therefore suppressed in .tsan-suppressions. Initialize the variable in bidirectional_transfer_loop() before any worker threads or processes are created. This patch removes the race and allows dropping the corresponding TSAN suppression. Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent a89346e commit 2b551df

2 files changed

Lines changed: 6 additions & 12 deletions

File tree

.tsan-suppressions

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
# A static variable is written to racily, but we always write the same value, so
88
# in practice it (hopefully!) doesn't matter.
99
race:^want_color$
10-
race:^transfer_debug$
1110

1211
# A boolean value, which tells whether the replace_map has been initialized or
1312
# not, is read racily with an update. As this variable is written to only once,

transport-helper.c

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1343,24 +1343,16 @@ int transport_helper_init(struct transport *transport, const char *name)
13431343
/* This should be enough to hold debugging message. */
13441344
#define PBUFFERSIZE 8192
13451345

1346+
static int transfer_debug_enabled = -1;
1347+
13461348
/* Print bidirectional transfer loop debug message. */
13471349
__attribute__((format (printf, 1, 2)))
13481350
static void transfer_debug(const char *fmt, ...)
13491351
{
1350-
/*
1351-
* NEEDSWORK: This function is sometimes used from multiple threads, and
1352-
* we end up using debug_enabled racily. That "should not matter" since
1353-
* we always write the same value, but it's still wrong. This function
1354-
* is listed in .tsan-suppressions for the time being.
1355-
*/
1356-
13571352
va_list args;
13581353
char msgbuf[PBUFFERSIZE];
1359-
static int debug_enabled = -1;
13601354

1361-
if (debug_enabled < 0)
1362-
debug_enabled = getenv("GIT_TRANSLOOP_DEBUG") ? 1 : 0;
1363-
if (!debug_enabled)
1355+
if (transfer_debug_enabled <= 0)
13641356
return;
13651357

13661358
va_start(args, fmt);
@@ -1630,6 +1622,9 @@ int bidirectional_transfer_loop(int input, int output)
16301622
{
16311623
struct bidirectional_transfer_state state;
16321624

1625+
if (transfer_debug_enabled < 0)
1626+
transfer_debug_enabled = getenv("GIT_TRANSLOOP_DEBUG") ? 1 : 0;
1627+
16331628
/* Fill the state fields. */
16341629
state.ptg.src = input;
16351630
state.ptg.dest = 1;

0 commit comments

Comments
 (0)