mirror of
https://github.com/git/git.git
synced 2026-06-24 08:48:31 +00:00
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>
This commit is contained in:
committed by
Junio C Hamano
parent
a89346e34a
commit
85704eda18
@@ -7,7 +7,6 @@
|
||||
# A static variable is written to racily, but we always write the same value, so
|
||||
# in practice it (hopefully!) doesn't matter.
|
||||
race:^want_color$
|
||||
race:^transfer_debug$
|
||||
|
||||
# A boolean value, which tells whether the replace_map has been initialized or
|
||||
# not, is read racily with an update. As this variable is written to only once,
|
||||
|
||||
@@ -1343,24 +1343,18 @@ int transport_helper_init(struct transport *transport, const char *name)
|
||||
/* This should be enough to hold debugging message. */
|
||||
#define PBUFFERSIZE 8192
|
||||
|
||||
static int transfer_debug_enabled = -1;
|
||||
|
||||
/* Print bidirectional transfer loop debug message. */
|
||||
__attribute__((format (printf, 1, 2)))
|
||||
static void transfer_debug(const char *fmt, ...)
|
||||
{
|
||||
/*
|
||||
* NEEDSWORK: This function is sometimes used from multiple threads, and
|
||||
* we end up using debug_enabled racily. That "should not matter" since
|
||||
* we always write the same value, but it's still wrong. This function
|
||||
* is listed in .tsan-suppressions for the time being.
|
||||
*/
|
||||
|
||||
va_list args;
|
||||
char msgbuf[PBUFFERSIZE];
|
||||
static int debug_enabled = -1;
|
||||
|
||||
if (debug_enabled < 0)
|
||||
debug_enabled = getenv("GIT_TRANSLOOP_DEBUG") ? 1 : 0;
|
||||
if (!debug_enabled)
|
||||
if (transfer_debug_enabled < 0)
|
||||
BUG("somebody forgot to check GIT_TRANSLOOP_DEBUG!");
|
||||
if (!transfer_debug_enabled)
|
||||
return;
|
||||
|
||||
va_start(args, fmt);
|
||||
@@ -1630,6 +1624,9 @@ int bidirectional_transfer_loop(int input, int output)
|
||||
{
|
||||
struct bidirectional_transfer_state state;
|
||||
|
||||
if (transfer_debug_enabled < 0)
|
||||
transfer_debug_enabled = getenv("GIT_TRANSLOOP_DEBUG") ? 1 : 0;
|
||||
|
||||
/* Fill the state fields. */
|
||||
state.ptg.src = input;
|
||||
state.ptg.dest = 1;
|
||||
|
||||
Reference in New Issue
Block a user