FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-145615: Fix mimalloc page leak in the free-threaded build by colesbury · Pull Request #145626 · python/cpython · GitHub

Repository navigation

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .c  (4) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed a memory leak in the :term:`free-threaded build` where mimalloc pages
could become permanently unreclaimable until the owning thread exited.
5 changes: 4 additions & 1 deletion Objects/mimalloc/heap.c
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
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@ static bool mi_heap_page_collect(mi_heap_t* heap, mi_page_queue_t* pq, mi_page_t
// note: this will free retired pages as well.
bool freed = _PyMem_mi_page_maybe_free(page, pq, collect >= MI_FORCE);
if (!freed && collect == MI_ABANDON) {
_mi_page_abandon(page, pq);
// _PyMem_mi_page_maybe_free may have moved the page to a different
// page queue, so we need to re-fetch the correct queue.
uint8_t bin = (mi_page_is_in_full(page) ? MI_BIN_FULL : _mi_bin(page->xblock_size));
_mi_page_abandon(page, &heap->pages[bin]);
}
}
else if (collect == MI_ABANDON) {
Expand Down
10 changes: 7 additions & 3 deletions Objects/mimalloc/page.c
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
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,13 @@ static void _mi_page_thread_free_collect(mi_page_t* page)

// update counts now
page->used -= count;

if (page->used == 0) {
// The page may have had a QSBR goal set from a previous point when it
// was all-free. That goal is no longer valid because the page was
// allocated from and then freed again by other threads.
_PyMem_mi_page_clear_qsbr(page);
}
}

void _mi_page_free_collect(mi_page_t* page, bool force) {
Expand All @@ -225,9 +232,6 @@ void _mi_page_free_collect(mi_page_t* page, bool force) {

// and the local free list
if (page->local_free != NULL) {
// any previous QSBR goals are no longer valid because we reused the page
_PyMem_mi_page_clear_qsbr(page);

if mi_likely(page->free == NULL) {
// usual case
page->free = page->local_free;
Expand Down
2 changes: 2 additions & 0 deletions Objects/mimalloc/segment.c
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
Original file line number Diff line number Diff line change
Expand Up @@ -1286,6 +1286,7 @@ static bool mi_segment_check_free(mi_segment_t* segment, size_t slices_needed, s
_mi_stat_decrease(&tld->stats->pages_abandoned, 1);
#ifdef Py_GIL_DISABLED
page->qsbr_goal = 0;
mi_assert_internal(page->qsbr_node.next == NULL);
#endif
segment->abandoned--;
slice = mi_segment_page_clear(page, tld); // re-assign slice due to coalesce!
Expand Down Expand Up @@ -1361,6 +1362,7 @@ static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap,
// if everything free by now, free the page
#ifdef Py_GIL_DISABLED
page->qsbr_goal = 0;
mi_assert_internal(page->qsbr_node.next == NULL);
#endif
slice = mi_segment_page_clear(page, tld); // set slice again due to coalesceing
}
Expand Down
31 changes: 25 additions & 6 deletions Objects/obmalloc.c
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
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,12 @@ should_advance_qsbr_for_page(struct _qsbr_thread_state *qsbr, mi_page_t *page)
}
return false;
}

static _PyThreadStateImpl *
tstate_from_heap(mi_heap_t *heap)
{
return _Py_CONTAINER_OF(heap->tld, _PyThreadStateImpl, mimalloc.tld);
}
#endif

static bool
Expand All @@ -159,23 +165,35 @@ _PyMem_mi_page_maybe_free(mi_page_t *page, mi_page_queue_t *pq, bool force)
#ifdef Py_GIL_DISABLED
assert(mi_page_all_free(page));
if (page->use_qsbr) {
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)PyThreadState_GET();
if (page->qsbr_goal != 0 && _Py_qbsr_goal_reached(tstate->qsbr, page->qsbr_goal)) {
struct _qsbr_thread_state *qsbr = ((_PyThreadStateImpl *)PyThreadState_GET())->qsbr;
if (page->qsbr_goal != 0 && _Py_qbsr_goal_reached(qsbr, page->qsbr_goal)) {
_PyMem_mi_page_clear_qsbr(page);
_mi_page_free(page, pq, force);
return true;
}

// gh-145615: since we are not freeing this page yet, we want to
// make it available for allocations. Note that the QSBR goal and
// linked list node remain set even if the page is later used for
// an allocation. We only detect and clear the QSBR goal when the
// page becomes empty again (used == 0).
if (mi_page_is_in_full(page)) {
_mi_page_unfull(page);
}

_PyMem_mi_page_clear_qsbr(page);
page->retire_expire = 0;

if (should_advance_qsbr_for_page(tstate->qsbr, page)) {
page->qsbr_goal = _Py_qsbr_advance(tstate->qsbr->shared);
if (should_advance_qsbr_for_page(qsbr, page)) {
page->qsbr_goal = _Py_qsbr_advance(qsbr->shared);
}
else {
page->qsbr_goal = _Py_qsbr_shared_next(tstate->qsbr->shared);
page->qsbr_goal = _Py_qsbr_shared_next(qsbr->shared);
}

// We may be freeing a page belonging to a different thread during a
// stop-the-world event. Find the _PyThreadStateImpl for the page.
_PyThreadStateImpl *tstate = tstate_from_heap(mi_page_heap(page));
llist_insert_tail(&tstate->mimalloc.page_list, &page->qsbr_node);
return false;
}
Expand All @@ -192,7 +210,8 @@ _PyMem_mi_page_reclaimed(mi_page_t *page)
if (page->qsbr_goal != 0) {
if (mi_page_all_free(page)) {
assert(page->qsbr_node.next == NULL);
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)PyThreadState_GET();
_PyThreadStateImpl *tstate = tstate_from_heap(mi_page_heap(page));
assert(tstate == (_PyThreadStateImpl *)_PyThreadState_GET());
page->retire_expire = 0;
llist_insert_tail(&tstate->mimalloc.page_list, &page->qsbr_node);
}
Expand Down
Loading

Back | FazBrowse Home | New Git URL