Defer releasing the iterator's reference to its iterable (free-threading)

Background

In free-threaded builds, a shared iterator advanced to exhaustion by multiple threads can double-Py_DECREF the iterable it holds, because the exhaustion path is not atomic:

it->it_seq = NULL;
Py_DECREF(seq);

Since adding atomic op is expensive, a better solution would be to defer releasing of the reference. This was already implemented for list and tuple in #128637. As that PR explains:

Tuple iteration could occasionally crash when the iterator’s reference to the
tuple was cleared on exhaustion. Like with list iteration, in free-threaded
builds we can’t safely and efficiently clear the iterator’s reference to the
iterable (doing it safely would mean extra, slow refcount operations), so just
keep the iterable reference around.

Problem

@devdanzin’s fusil-based testing reports that other built-in iterables have not received this fix and still crash. For example, memoryview reliably aborts on a free-threaded debug build

Iterables that still need the same treatment are:

  • memoryviewmemoryiter_next
  • bytesstriter_next
  • strunicodeiter_next / unicode_ascii_iter_next
  • setsetiter_iternext
  • dictdictiter_iternext_threadsafe

Proposal

I suggest using similar approch to fix iterables above, like those for list and tuple. In fact, some already have an issue (thank devdanzin for opening them) or PR, some don’t, and one PR has been open for a long time. I suggest better track them here:

Iterable Status Link
set PR open for a long time, not merged #144357
dict issue filed, claimed, no PR yet #154130
memoryview discussed in a comment, claimed, no PR yet #124397
bytes
str
3 Likes

My suggestion is 1/ Expand set issue to include all the builtin iterators (closing the dict issue). Multiple PRs are OK. 2. Request reviews from the 4 coredevs involved in the list/tuple pr (mpage is not one).

EDIT: the list/tuple PR added some TODO’s. Ask if these should be done before doing more iterators.

1 Like

Thanks for the reminder. I just checked the list/tuple PR — indeed 4 TODOs left, luckily they’ve all been resolved. Next I’ll look at the set issue and try to track all the builtin iterators there.

1 Like