gh-123271: Make builtin zip method safe under free-threading

[eendebakpt]

In this PR we make zip safe under free-threading.

• Issue: Builtin zip method is not safe under free-threading #123271

[Eclips4]

Suggested change

	Make :meth:`zip` safe under free-threading.
	Make :func:`zip` thread-safe without the :term:`GIL`.

[eendebakpt]

Even with this PR the zip function is not thread-safe: when iterating over zip(range(100), range(100)) one can obtain a tuple (2,3). What does PR (hopefully) does is modifying the code so that the interpreter does not crash. For this reason I would like to be careful with the term "thread-safe". I choose not to make zip fully thread safe because of the performance impact, see the discussion in #120496

[Eclips4]

Thanks for clarifying. The word safe is a bit confusing, I guess a lot of other people would read it as thread-safe. I think we need to clarify the reason why zip isn't safe in free-threaded build.

[colesbury]

I think we want to enable re-use under certain conditions. We should probably put the logic in some internal function, possibly in pycore_object.h. I think the condition for the free-threaded build is:

1. ob_tid matches _Py_ThreadId()
2. ob_ref_local is 1
3. ob_ref_shared is 0

The logic is that no other thread calling zip_next can re-use lz->result because of the thread id condition (condition 1). And no thread outside of zip_next can incref that object because the combined conditions ensure that lz->result is the only reference.

[eendebakpt]

@colesbury Thanks for the hint! I implementeded the suggestion

• I placed the logic in a single method _Py_Reuse_Immutable_Object so we can use the trick code for several other cases (e.g. itertools.pairwise, enumerate). The location and name of the new method can be changed though
• I re-ordered the three conditions because I suspect that ob_ref_local is most informative and fastest to check
• I am now checking ob_ref_shared to be zero, but perhaps we should check on Py_ARITHMETIC_RIGHT_SHIFT(Py_ssize_t, ob_ref_shared , _Py_REF_SHARED_SHIFT)?

[colesbury]

Thanks @eendebakpt. The check that ob_ref_shared is entirely zero is correct as you wrote it. We don't want non-zero flags in this case as that could allow another thread to concurrently incref the object in some circumstances.

[colesbury]

The preferred style is to use for loops instead of using list comprehensions for side effects.

[colesbury]

We run lots of unit tests in CI, so they individually need to be very fast. For the free-threaded tests, we should aim for <0.1 seconds on two cores (i.e., when run with taskset -c 0-1 on Linux)

[eendebakpt]

On my system (windows) the test is 0.07 seconds for the free-threading build. I will reduce the number of threads and iterations a bit so it is even faster.

[colesbury]

Thanks for the fix @eendebakpt!

There's maybe a dozen or so similar patterns in CPython and has_unique_reference in dictobject.c. Would you be interested in updating them to use _PyObject_IsUniquelyReferenced?

I searched for the regex Py_REFCNT.* == 1, but I probably wouldn't change the matching assert() statements.

[eendebakpt]

@colesbury Thanks for the guidance here! I was indeed planning on addressing some of the similar patterns. I'll open an new issue for this.