GeezerVison

Thanks for the review, Sergey!

But I have to say that I've discovered one even more difficult case without tracing at all:

import _thread
import threading

local = _thread._local()

class ClassWithDel:
    def __del__(self):
        getattr(local, 'c', None)

def thread_func():
    local.c = ClassWithDel()

t = threading.Thread(target=thread_func)
t.start()
t.join()

There's a leak with ASAN since finalizer of local.c object recreates localsdict for this thread.

CC @mpage @colesbury @ambv as author and reviewers of #121655.

Btw, we can merge this as is and create another issue for this case if this will be clearer.

This does not look like the right approach to me. I think you will end up playing whac-a-mole where each time you "fix" one leak you will introduce other bugs. You need a robust approach to dealing with finalizers like the GC has.

Thanks for the comment, @colesbury .
I'd like to stress that this PR doesn't introduce new bugs, it just doesn't fix all related issues in the current state. There's a leak with this code snippet on main.

And I'll be grateful for some piece of advice, if possible.

I think you just haven't yet found the new bugs that this PR introduces. The original bug report was only found via fuzzing. I suspect that if you keep fuzzing you will find different ones with this PR.

The problems is that we have code in the form:

Py_CLEAR(tstate->var1);
Py_CLEAR(tstate->var2);
...
Py_CLEAR(tstate->varN);

The underlying problem is that when you clear tstate->var2 you may execute Python code that re-initializes tstate->var2 or variables cleared earlier like tstate->var1. If you move around the order of clearing, you change the possible bugs.

For example, if you move tstate->threading_local_key to the end, you now have potential leaks where thread local variable destructors set things like tstate->asyncio_running_loop

In general, I agree with you, swapping clear function calls isn't good way of fixing anything.
I have some new thoughts on this topic. I'll continue to find better solution.

Friendly ping, @colesbury @vstinner @mpage

This PR is stale because it has been open for 30 days with no activity.

This PR is stale because it has been open for 90 days with no activity.