Release Python Global Interpreter Lock During Detection

[Jgunde]

Summary:
This PR releases Python’s Global Interpreter Lock (GIL) during the CPU-intensive detection stage. This allows other threads in the Python process to run concurrently.

Details:

• The GIL prevents multiple Python threads from executing in parallel. If the GIL is not released, detection blocks the entire Python process, starving other threads of CPU.
• On low-resolution images or with high decimate values, detection is fast (milliseconds), so blocking is minimal. With high-res images or low decimate values, detection can take up to a second, making GIL release critical for multi-threaded applications.
• Manual GIL release can be unsafe if C code interacts with Python objects or is not thread-safe. Here, this is not an issue because apriltag_detector_detect() is implemented purely in C and does not interact with Python objects.

References:
Python docs: Releasing the GIL from extension code

Testing:
Changes are tested and work as intended.

[christian-rauch]

I am not familiar with handing the GIL manual. Can this cause issues when you run apriltag_detect in Python in multiple threads using the same detector?

[Jgunde]

I am not familiar with handing the GIL manual. Can this cause issues when you run apriltag_detect in Python in multiple threads using the same detector?

@christian-rauch
I didn't consider this use case. Since detection can run on multiple cores, I assumed it would only be used in a single thread as multithreading wouldn't provide a performance benefit.

I just finished some multithreaded testing and your hypothesis is correct. The program crashed when using the same detector object across multiple threads.

I do think releasing the GIL provides a large enough benefit to warrant its inclusion. Maybe we add a note that the detect method is not thread safe?

For those that do want to use a single detector object across multiple threads, they can use the Python threading module's lock objects. It's common practice to using locks in this way to synchronize access to shared objects.
Personally, it seems simpler to create a distinct detector object for each thread.

It also seems possible to implement the lock in C using the CPython lock interface.

What do you think? Would updating the Python documentation noting that the detect method is not thread safe be enough?

[christian-rauch]

I just finished some multithreaded testing and your hypothesis is correct. The program crashed when using the same detector object across multiple threads.

Thanks for testing this. Just to make sure, it does not crash with the commit just before your PR? Do you have a PoC to reproduce this with an example image?

What do you think? Would updating the Python documentation noting that the detect method is not thread safe be enough?

Since we cannot know how users of the library use the bindings, this could now potentially break code in a hard to debug way. Adding documentation would only help new users of the bindings, if they read it at all.

Is it possible to add this manual locking inside apriltag_detect?

[Jgunde]

Thanks for testing this. Just to make sure, it does not crash with the commit just before your PR?

Correct. It does not crash when using the same detector object across multiple threads.

Do you have a PoC to reproduce this with an example image?

I don't have an example image. In Python, I simply created a detector, detector = apriltag("tagStandard41h12"), and ran detection, detector.detect(image), in two threads simultaneously. I got this output:

realloc(): invalid next size
Aborted

Is it possible to add this manual locking inside apriltag_detect?

I think so. It'll probably require 10-30 lines of code. I'll look into this.

[Jgunde]

Hi Christian,

I've created a PR here that implements a lock and acquires it before running detection. This prevents the crash mentioned earlier in this thread.

Best,
Jacob