Let critical_section work with pymutex#7031
Conversation
Based on python/cpython#135899 this'll be supported in 3.14 (and can be backported to 3.13 fairly easily).
That PR got backported to 3.14.0rc1, which was released before you opened this PR? 🤔 |
Yes I know about that now (I hadn't seen rc1 when I posted this). I'll get this finished fairly soon |
scoder
left a comment
scoder
left a comment
There was a problem hiding this comment.
Looks good, just style comments.
| threads = [ | ||
| threading.Thread(target=worker) | ||
| for _ in range(n_threads) | ||
| ] | ||
| for t in threads: | ||
| t.start() | ||
| for t in threads: | ||
| t.join() |
There was a problem hiding this comment.
This is used so often, it seems worth a helper function.
| mutex = "Mutex" if self.critical_section.is_pymutex_critical_section else "" | ||
|
|
||
| code.putln( | ||
| f"__Pyx_PyCriticalSection_End{self.len}(&{variable_name});" | ||
| f"__Pyx_PyCriticalSection_End{mutex}{self.len}(&{variable_name});" |
There was a problem hiding this comment.
Should we use two different macro name suffixes for the two cases, Object and Mutex? They seem equivalent and the code already sets the suffix to the empty string explicitly, so it seems easy to do.
There was a problem hiding this comment.
I think I'd rather clean up the macro names a bit to match what CPython uses internally - I've diverged a little and I'm regretting it a little. (That doesn't use Object)
3 participants
Based on python/cpython#135899 this'll be supported in 3.14 (and can be backported to 3.13 fairly easily).
Draft for now because I can't easily test this until the next Python release