Do not release the GIL during object destruction

Currently, the GIL can be released during object deallocation, either because a C function explicitly does so, or the object has a Python __del__ method. This adds a sharp edge to reasoning about thread-safety with the GIL.

I proposed not allowing context switches during object finalization to simplify the with-GIL concurrency model.

This code is thread-safe with the GIL (g is a global variable)

temp = fancy_object()
g = (1, 2)
g += (3, 4)
# Expect g to be (1,2,3,4) here

but this is not:

temp = fancy_object()
g = (1, 2)
temp = (3, 4)
g += temp
# g might be (3,4) here if another thread sets g to ()

because the assignment temp = (3, 4) code cause fancy_object() to be destroyed and it might have a finalizer.

This is only a problem with CPython; MicroPython and GraalPython have tracing GCs, so only collect objects at safe points.

If we disallow context switching during implicit calls to finalizers, this problem goes away.

The only downside, AFAICT, is that programs that rely on finalizers to clean resources like sockets, and were parallel and relied on running other threads while sockets were being released would see reduced parallelism under the GIL.

Of course, the upside is a reduction in hard to detect race conditions in existing Python programs using the GIL.

Thanks to @pitrou for highlighting this issue: PEP 805: Safe Parallel Python - #79 by pitrou

What’s the plan to make this work in C code? If someone calls a C function that internally uses Py_BEGIN_ALLOW_THREADS or something, then that will also release the GIL and cause the same problem.

It is almost trivial to implement as the GIL is reentrant. We just bump the counter on the GIL around the call to finalizers.

Some destructors can wait for other operations to finish in other threads, so that can’t work in the general case unfortunately.

(it’s not a terrific practice, but it’s sometimes hard to avoid)

Do you have an example?

There is no guarantee as to which, if any, other threads run when the GIL is released, so that might not work reliably.

Yeah, but that can lead to deadlocks. For example:

  1. There is a mutex in C that is sometimes used in an object’s finalizer (the stderr lock, for example).
  2. Thread A wants to acquire the mutex, so it calls Py_BEGIN_ALLOW_THREADS, releasing the GIL and acquiring the mutex.
  3. Thread B acquires the GIL, and then deallocates an object that acquires that same mutex. It tries to release the GIL via Py_BEGIN_ALLOW_THREADS, but it’s not actually released because we’re in a finalizer. It then starts waiting on the mutex.
  4. Thread A, holding the mutex, enters Py_END_ALLOW_THREADS, now waiting on the GIL.
  5. Thread B is waiting on the mutex while holding the GIL, while thread A holds the mutex and is waiting on the GIL. Lock-ordering deadlock!

The first example I could find in CPython is IsolatedAsyncioTestCase:

whose destructor calls asyncio.Runner.close which waits for a thread pool to shutdown:

That’s just buggy code, using the GIL as a mutex.

thread A:

  • releases the GIL
  • acquires mutex M
  • acquires the GIL

thread B

  • acquires the GIL
  • releases the GIL
  • acquires mutex M

This is already prone to deadlock as the GIL is re-entrant.

That’s applicable to any multi-threaded code that runs under a GIL-enabled Python.

That’s horrible.
Why not use the tearDown() method?

So this does have an impact, but given the examples so far, I still think it is an improvement on what we have.

I have no idea, perhaps this can be investigated using git blame.

Other instances of __del__ in Dask Distributed and PySpark definitely seem to involve some multi-threaded synchronization (or IO on a more general basis).

Well, I wanted to point out that it will break some existing uses.

(note that the same remark about destructors potentially releasing the GIL applies to other impromptu-running code, such as weakref finalizers)

Yes, you need consistent lock ordering. My point is that making Py_BEGIN_ALLOW_THREADS not actually release the GIL sometimes will result in inconsistent lock ordering, and thus deadlocks.

1 Like

Your first example can be made to have data races even with the GIL by implementing __iadd__ on the fancy_object in a way that the addition happens in two steps and releases the GIL between them. Is that pathological? I would say no. It’s perfectly reasonable for a 3rd party type to document that it does not incur the cost of synchronization, and it’s up to users using concurrency to provide synchronization.

It’s fundamentally incorrect for people to use the GIL to protect their code from data races without taking into account implementation details.

The GIL only protects the interpreter state and ensures it is consistent; It does not guarantee for any specific arbitrary operation like += that there are no data races, only that at each context switch, all threads have the same state. I believe misconceptions about what the GIL actually protects are part of why adoption of FT has been so slow. The obvious answer here is if you know you are sharing a value across threads with concurrent use and that at least one of the threads is modifying the value, you need to provide synchronization.

FT’s per-object/per object pair locks work exceptionally well for this for those who have already internalized what’s actually required to prevent data races in application code.

edit: sorry, not fancy object implementing iadd, but having g be of a type that does.

It’s fundamentally incorrect for people to use the GIL to protect their code from data races without taking into account implementation details.

If people are unable to tell whether their code is safe from reading the docs, shouldn’t we be fixing the docs, rather than making judgemental statements?

The GIL only protects the interpreter state and ensures it is consistent; It does not guarantee for any specific arbitrary operation like += that there are no data races.

The GIL does guarantee that += is atomic for the types you’re likely to use += on. Maybe you think that it shouldn’t, but that doesn’t change the fact that it does.
In

global x
x = some_int_value()
x += 2

the x += 2 is atomic with the GIL in CPython.

I believe misconceptions about what the GIL actually protects are part of why adoption of FT has been so slow.

If that is how the GIL behaves, then they aren’t misconceptions

FT’s per-object/per object pair locks work exceptionally well for this for those who have already internalized what’s actually required to prevent data races in application code.

How is this relevant to the original suggestion?
Are you saying we should or shouldn’t make the suggested change?

It isn’t the GIL that guarantees this, it’s the implementation details of int interacting with the GIL.

By all means, make the docs clearer, but I don’t think we should be telling users that the GIL does things it doesn’t. It happens that for some types, users automatically get some safety, but that’s not something we can document as a property of the GIL, we have to document it as an implementation detail of specific types.

It was a tangential note given that this discussion is split off from another. In terms of what it means for this suggestion, I believe the appropriate answer is to make zero changes to the interpreter and document things in a way that users will get the correct result no matter which build they are using; That means telling users they need to provide synchronization in cases where they are unwilling to rely on specific implementation details, and could possibly include listing some useful implementation details that users may choose to rely upon, but that they should know are implementation details.

3 Likes

Not meant as a criticism, but this feels like an example of people in this topic talking past each other

(emphasis added)

That’s not very interesting, though, unless your mutable state consists of a single value. As soon as your mutable state is slightly more complicated, you have to be careful about synchronization, regardless of whether individual ops are atomic or not.

1 Like

I’m afraid I’m with Liz here - I don’t understand how the GIL guarantees that the += operation on integers is atomic. Isn’t it the code for the += operation that determines that?

Maybe what Mark means is that the code for += doesn’t do its own locking, and assumes it won’t be interrupted by other threads - which is only a valid assumption if the GIL is stopping other Python threads running? That’s fair, and to an extent I think it’s nitpicking to worry about whether it’s “the GIL” or “the += code” that’s providing atomicity in that case. But what does matter to a user is whether the operation is atomic - and that’s not a property of the GIL alone. After all, the += code could be updated to use its own locks under the free-threaded build.

But this seems to have gone a long way from the original proposal of not releasing the GIL during object destruction.

The fact is, people can write code that will be broken (in the sense of having data races or maybe deadlocks) if the proposed change is made. Maybe that code is written incorrectly. Maybe it should be fixed. But that doesn’t alter the fact that it probably exists - people write bad code (I know I do!!!)

Is it OK to break such code without a deprecation cycle? I’m not sure. If we do need a deprecation cycle, will that make the expected benefit (clearer thread-safety guarantees for the GIL-enabled build) less useful? That question feels quite “political”, with the answer depending on whether you’re optimistic about free-threaded Python. Personally, I don’t have a good feel for an answer here, either.

1 Like

If I read correctly this is a kind of sequential consistency / memory visibility guarantee?(without which I don’t think external synchronization will be even possible in the first place).

The need for external synchronization will be apparent for users coming from other multithreaded programming languages such as C++ and Java, but for those who use Python as their starting point (AFAIK the number of which are increasing :slight_smile: ) the possibility of some += being a gotcha might be less intuitive (well almost nothing is intuitive in concurrent programming). And I assume the number of users who will run experiments +=ing ints, strs and lists and other builtins in multiple threads just to see which types will break and which others won’t will be much smaller than the above number of first-time users.

It’s really related. The same reason why __del__ can be problematic based on a specific implementation of __del__ is the same reason why the += example isn’t general, but specific to how the types are implemented.

I don’t think there’s any better option here than just making sure users get good guidance on how to spot potential data races, and how to add synchronization where needed.

I think the bigger question here is whether or not this is even the job of the GIL. I don’t think it is, but I’m also comfortable enough with concurrency that I don’t find the issues here hard to navigate. I wish I were a better technical communicator, but I don’t think I’m the right person to write documentation of this, even though I think better documentation is the answer here.

In plain terms, with the GIL enabled build, and barring any “evil” manipulations of underlying process memory that are unreasonable to design for, the interpreter state is guaranteed to be consistent at any given point where the interpreter switches threads. The GIL does not provide any documented guarantee of the order in which threads run or when they are allowed to context switch, though as far as the implementation details go, there are some guarantees that must exist that can be inferred.

Along these lines, as someone optimistic about freethreading and who has adopted it in a professional setting, I don’t see much value in documenting implementation details related to the GIL for people to rely upon, because users relying upon such implementation details will have a harder time with freethreading adoption.

While that’s a “political” opinion, the current accepted trajectory for CPython is that it is a goal to eventually remove the GIL.