gh-111569: Implement Python critical section API #111571
Conversation
Critical sections are helpers to replace the global interpreter lock with finer grained locking. They provide similar guarantees to the GIL and avoid the deadlock risk that plain locking involves. Critical sections are implicitly ended whenever the GIL would be released. They are resumed when the GIL would be acquired. Nested critical sections behave as if the sections were interleaved.
|
@ericsnowcurrently I'd appreciate your review if you have time to look at this. This builds off of the |
| }; | ||
| assert(test_data.obj1 != NULL); | ||
| assert(test_data.obj2 != NULL); | ||
| assert(test_data.obj3 != NULL); |
There was a problem hiding this comment.
Just a drive-by comment, I would much prefer to have conditional returns from native test functions than asserts. When these fail, they can block test progress (on Windows it pops up a modal dialog by default), or may trigger error reporting or a debugger. It also shows up as a test crash, rather than a failure.
Perhaps a new macro is needed to print location information to the console and then fail?
There was a problem hiding this comment.
I agree that the failure case is unpleasant, but I don't think trying to route proper error returns in the C API tests is worthwhile. To the extent that there are bugs in the tested code, they are likely to show up as crashes, deadlocks, or other unpleasant behavior even if we make the asserts more nicely behaved (because this is testing C code, not Python). Additionally, it's challenging to route error messages from other threads in C since they don't always have a GIL context and might not be able to create Python error objects.
One consequence of this is that I think we should basically have a zero tolerance for flaky C API tests. If we are going to be bothered by an unpleasant failure, it better signify something important.
If crashes in the C API tests become an actual problem, then I think it would be better to invest in running these tests in a subprocess like we do with test_support.script_helper.assert_python_failure.
There was a problem hiding this comment.
Yeah, that's reasonable. I'm mostly annoyed about it because some versions of MSVC have a compiler bug on ARM64 that causes some of the interlocked tests to fail, which means the entire test run is basically useless until all the machines get updated (as far as I can tell, the code is only ever tested and never actually used, so it doesn't need fixing for our releases - there's nothing we can do about it on our side anyway).
Critical sections are helpers to replace the global interpreter lock with finer grained locking. They provide similar guarantees to the GIL and avoid the deadlock risk that plain locking involves. Critical sections are implicitly ended whenever the GIL would be released. They are resumed when the GIL would be acquired. Nested critical sections behave as if the sections were interleaved.