bpo-44991: Make GIL handling more explicit in `sqlite3` callbacks by erlend-aasland · Pull Request #27934 · python/cpython

erlend-aasland

acquire the GIL at the very start[1]
release the GIL at the very end

[1] The trace callback performs a sanity check before acquiring the GIL

https://bugs.python.org/issue44991

Automerge-Triggered-By: GH:encukou

Exception: in the trace callback, do a sanity check before attempting to
hold the GIL.

FYI, I'll normalise the callback names in a separate PR.

_destructor is also a callback function. It should get & release the GIL itself. (The NULL check can be moved here as well, as other calls of free_callback_context are definitely with a non-NULL context.)
PyGILState_Ensure/PyGILState_Release can be then removed from both create_callback_context and free_callback_context. Those will always be called with the GIL held.

Erlend E. Aasland added 2 commits

August 31, 2021 13:45

- since _destructor is a callback, wrap it in GIL lock/unlock
- remove GIL lock/unlock from create_callback_context()

_destructor is also a callback function. It should get & release the GIL itself. (The NULL check can be moved here as well, as other calls of free_callback_context are definitely with a non-NULL context.)
PyGILState_Ensure/PyGILState_Release can be then removed from both create_callback_context and free_callback_context. Those will always be called with the GIL held.

Yes, that's cleaner. Thanks! See d42eb38.

PTAL :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

Thanks, and thank you for reviewing!