Note that I found this while investigating #118138, however I think it is a separate bug that should be addressed under a separate issue.

CC @cmaloney who seems to be looking at related issues in this area already 👋

I'm pretty sure we don't support concurrent calls to __next__. You need to manually serialize the calls yourself. See gh-124397.

See gh-124397.

That is a very interesting discussion, thanks for linking!

I'm pretty sure we don't support concurrent calls to __next__. You need to manually serialize the calls yourself.

I totally agree that in general iterators should not be assumed to be thread-safe, in the broader sense. However they should not crash, which is happening here. In the description I reported an assertion failure, but the repro script will crash with assertions disabled too, just in more "interesting" ways.

You can get similar crashes from pure-Python by concurrently calling an object's __init__. I don't think there's much we can do without substantially hurting performance.

I'm sure there are places where fixing crashes without substantially hurting performance might be difficult or impossible, but I don't think this is one of them. It should have no impact on GIL performance and minimal in free-threading.

The third bullet point in #124397 is:

Other iterators implemented in C will get only the minimal changes necessary to cause them to not crash in a free-threaded build. The edits should be made in a way that does not impact existing semantics or performance (i.e. do not damage the standard GIL build).

I think this fix is exactly that...or at least, that was exactly what I intended when I wrote it. Of course, I may well have missed something!

I'll let others weigh in here, but my personal stance is that concurrent iteration without synchronization by the caller is not a real use-case. The example you sent would suffer from so much lock contention that it would be no better than calling it in a single thread :(

I think that last point is in regards to making the iterator thread-safe when others are calling separate methods on it, not necessarily concurrent calls to __next__. I think this needs clarification, though.

From looking at the repro code, this looks like something we should fix.

Ok, thanks Sam. For clarity: when should we fix these cases?

This looks like concurrent operations on a NamedTemporaryFile. I think I/O should generally be internally synchronized. That's mostly true for Python already, it's usually true at the OS level, as well as in other language runtimes.

I get that there is a crash in iteration, but it doesn't look like there's an iterator shared between threads.

The NamedTemporaryFile is the iterator, and that's shared between threads.

Thanks. That still feels pretty different from list and dict iterators. For one thing, it'd be difficult to make list and dict iterators thread-safe without slowing down the common case substantially. I don't think that's the case here. The iteration interface just provides access to readline, which is already synchronized.

I think there's a difference between objects like NamedTemporaryFile that provide an iteration interface and objects like list_iterator, which are usually temporary objects that you don't think about and rarely store to variables.

Ok, that makes sense to me.