On Mon, Jul 10, 2017 at 09:24:18AM -0400, Jeff King wrote:
> You can also try:
>
> make SANITIZE=thread test
>
> but it's not clean. I poked at some of the errors, and I don't think
> there a problem in practice (though they may be worth cleaning up in the
> name of code hygiene).
Here's an example that I fixed up long ago, but never quite had the
stomach to seriously propose.
If it were the last one, it might be worth applying to get a clean run
of "make test", but I think it's just one of many.
-- >8 --
Date: Mon, 8 Dec 2014 03:02:34 -0500
Subject: [PATCH] transport-helper: initialize debug flag before starting
threads
The transport_debug() function uses a static variable to
lazily cache the boolean value of $TRANSPORT_DEBUG. If a
program uses transport-helper's bidirectional_transfer_loop
(as remote-ext and remote-fd do), then two threads may both
try to write the variable at the same time.
We can fix this by initializing the variable right before
starting the threads.
Noticed by "-fsanitize=thread". This probably isn't a
problem in practice, as both threads will write the same
value, and we are unlikely to see a torn write on an "int"
(i.e., on sane platforms a write to an int is atomic).
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
transport-helper.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 33cff38cc..76f19ddb0 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1113,17 +1113,23 @@ int transport_helper_init(struct transport *transport, const char *name)
/* This should be enough to hold debugging message. */
#define PBUFFERSIZE 8192
+static int transport_debug_enabled(void)
+{
+ static int debug_enabled = -1;
+
+ if (debug_enabled < 0)
+ debug_enabled = getenv("GIT_TRANSLOOP_DEBUG") ? 1 : 0;
+ return debug_enabled;
+}
+
/* Print bidirectional transfer loop debug message. */
__attribute__((format (printf, 1, 2)))
static void transfer_debug(const char *fmt, ...)
{
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 (!transport_debug_enabled())
return;
va_start(args, fmt);
@@ -1292,6 +1298,10 @@ static int tloop_spawnwait_tasks(struct bidirectional_transfer_state *s)
pthread_t ptg_thread;
int err;
int ret = 0;
+
+ /* initialize static global debug flag as a side effect */
+ transport_debug_enabled();
+
err = pthread_create(>p_thread, NULL, udt_copy_task_routine,
&s->gtp);
if (err)
--
2.13.2.1071.gcd8104b61
