TSan: data race in pending calls about cpython HOT 6 CLOSED

@colesbury @swtaarrs @corona10

from cpython.

@swtaarrs, I don't think you need to annotate PyMutex. I think it's more likely something like sem_clockwait not being supported by TSan.

colesbury/nogil-3.12@7507a77

from cpython.

Oh, I missed that. I'm curious if annotations for PyMutex fix this, but it looks to me like a data race:

T1: Writes pending->calls_to_do (nonatomic, but holding pending->mutex);
T2: Reads pending->calls_to_do (atomic, but not holding pending->mutex);

Either T1 needs an atomic write or T2 needs to hold pending->mutex.

from cpython.

(heh, I saw your comment pop up as I clicked to post mine. sounds like we agree!)

TSAN still doesn't like the nonatomic increment even with PyMutex annotated (I verified that the annotations are working, since the error message now says that it holds the relevant PyMutex during the bad write).

I'm going back to my initial thought was that this is a real data race. We shouldn't be mixing nonatomic writes with atomic reads, even if it works out in practice. Changing the increment and decrement of pending->calls_to_do to atomic gets rid of the error, and I wouldn't expect it to be a performance issue.

from cpython.

It looks like the root of the issue here is that tsan doesn't know about PyMutex, which is used to protect the data member being mutated here. I believe I've found the set of function calls used to annotate mutex implementations, so I'll work on adding those to PyMutex.

from cpython.

@colesbury That's already in main. Any other ideas? The annotations look pretty straightforward so I'll give them a try.

from cpython.