Re: tsan: t5400: set_try_to_free_routine

From: Martin Ågren <martin.agren@gmail.com>
Date: Aug 15, 2017, 18:44 UTC
Message-ID: <CAN0heSpPihOcXgP-iCkBFvvP2x-8=oxYGUFJbUhF8mwcYfgoxA@mail.gmail.com>
In-Reply-To: <CAGZ79kYbPvmqV1PR_NjxLbp2vqC6=JdP+-hzdwQvUmVLZU_Wtg@mail.gmail.com>

On 15 August 2017 at 19:35, Stefan Beller <sbeller@google.com> wrote:

Show 19 quoted lines

> On Tue, Aug 15, 2017 at 5:53 AM, Martin Ågren <martin.agren@gmail.com> wrote:
>> Using SANITIZE=thread made t5400-send-pack.sh hit the potential race
>> below.
>>
>> This is set_try_to_free_routine in wrapper.c. The race relates to the
>> reading of the "old" value. The caller doesn't care about the "old"
>> value, so this should be harmless right now. But it seems that using
>> this mechanism from multiple threads and restoring the earlier value
>> will probably not work out every time. (Not necessarily because of the
>> race in set_try_to_free_routine, but, e.g., because callers might not
>> restore the function pointer in the reverse order of how they
>> originally set it.)
>>
>> Properly "fixing" this for thread-safety would probably require some
>> redesigning, which at the time might not be warranted. I'm just posting
>> this for completeness.
>
> Maybe related read (also error handling in threads):
> https://public-inbox.org/git/20170619220036.22656-1-avarab@gmail.com/

Thanks for the pointer. Threading is tricky business, indeed. I still haven't entirely groked all the users of set_try_to_free_routine, i.e., what they want to avoid and what they want to achieve. I'm just happy that the only user which cares about the old value is not threaded, to the best of my understanding.