py/gc: Make gc_lock_depth have a count per thread. #7238
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Should fix #6957. |
This commit makes gc_lock_depth have one counter per thread, instead of one global counter. This makes threads properly independent with respect to the GC, in particular threads can now independently lock the GC for themselves without locking it for other threads. It also means a given thread can run a hard IRQ without temporarily locking the GC for all other threads and potentially making them have MemoryError exceptions at random locations (this really only occurs on MCUs with multiple cores and no GIL, eg on the rp2 port). The commit also removes protection of the GC lock/unlock functions, which is no longer needed when the counter is per thread (and this also fixes the cas where a hard IRQ calling gc_lock() may stall waiting for the mutex). It also puts the check for `gc_lock_depth > 0` outside the GC mutex in gc_alloc, gc_realloc and gc_free, to potentially prevent a hard IRQ from waiting on a mutex if it does attempt to allocate heap memory (and putting the check outside the GC mutex is now safe now that there is a gc_lock_depth per thread). Signed-off-by: Damien George <damien@micropython.org>
The RP2040 has 2 cores and supports running at most 2 Python threads (the main one plus another), and will raise OSError if a thread cannot be created because core1 is already in use. This commit adjusts some thread tests to be robust against such OSError's. These tests now pass on rp2 boards. Signed-off-by: Damien George <damien@micropython.org>
b63767a to
4cdcbdb
Compare
Thanks. It does not hurry. Take your time. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an alternative to #7217.
This PR makes gc_lock_depth have one counter per thread, instead of one global counter. This makes threads properly independent with respect to the GC, in particular threads can now independently lock the GC for themselves without locking it for other threads. It also means a given thread can run a hard IRQ without temporarily locking the GC for all other threads and potentially making them have MemoryError exceptions at random locations (this really only occurs on MCUs with multiple cores and no GIL, eg on the rp2 port).
The PR also removes protection of the GC lock/unlock functions, which is no longer needed when the counter is per thread (and this also fixes the cas where a hard IRQ calling gc_lock() may stall waiting for the mutex).
It also puts the check for
gc_lock_depth > 0outside the GC mutex in gc_alloc, gc_realloc and gc_free, to potentially prevent a hard IRQ from waiting on a mutex if it does attempt to allocate heap memory (and putting the check outside the GC mutex is now safe now that there is a gc_lock_depth per thread).