gh-109593: ResourceTracker.ensure_running() calls finalizers

[vstinner]

multiprocessing: Reduce the risk of reentrant calls to ResourceTracker.ensure_running() by running explicitly all finalizers before acquiring the ResourceTracker lock.

• Issue: test_multiprocessing_forkserver.test_processes: ResourceTracker.ensure_running() calls itself and hangs indirectly via the GC #109593

[pitrou]

Hmm, but _run_finalizers should only be run at process end.

[vstinner]

Oh, test_asyncio.test_events fails with this change: test_get_event_loop_new_process() fails.

[vstinner]

Hmm, but _run_finalizers should only be run at process end.

What is a process end?

[pitrou]

Sorry, I meant interpreter shutdown.

[vstinner]

Hmm, but _run_finalizers should only be run at process end.

Oh in fact, what I want is to trigger a GC collection: I fixed my PR for that.

I confirm that my change fix #109593 (comment) reproducer.

@pitrou: Would you mind to review this updated fix?

[pitrou]

Ok, this is better, but the problem is that gc.collect is slow and ensure_running is called indirectly when instantiation a multiprocessing lock (see SemLock.__init__).

Instantiating a semaphore is currently 500x faster than a GC collection, and that's an optimistic measurement without a lot of objects allocated:

>>>%timeitgc.collect()
10.7ms ± 352µsperloop (mean ± std. dev. of7runs, 100loopseach)
>>>%timeitmp.Semaphore()
21.7µs ± 395nsperloop (mean ± std. dev. of7runs, 10,000loopseach)

[vstinner]

Ok, this is better, but the problem is that gc.collect is slow and ensure_running is called indirectly when instantiation a multiprocessing lock (see SemLock.init).

Do you want to propose a different fix?

My concern is that currently, multiprocessing can hang randomly. IMO it's bad and must be fixed. I prefer a slow multiprocessing than a multiprocessing which hangs randomly.

Also, I have limited interest in developing the most efficient fix. So if you have free cycles, please go ahead :-)

These days, I'm busy fixing tons of buildbot failures:

• https://buildbot.python.org/all/#/release_status
• https://discuss.python.org/t/report-of-buildbots-and-github-actions-status-september-2023/33725/1

Obviously, I would be fine with a fast and correct fix for this issue :-)

[pitrou]

Well, TBH, I'm not sure this issue is very common as I don't think I have every seen it elsewhere.
It does deserve fixing, but is not that urgent either (at least not users of Python :-)).

But, yes, I'll try to come up with a fix.

[vstinner]

Well, TBH, I'm not sure this issue is very common as I don't think I have every seen it elsewhere.

It makes fail more and more buildbots, so for me, it's an urgency.

What I mean in my previous message is that I'm considering to fix the issue right now, and we can have time later to revisit the issue and find a better fix (with lower impact on performance).

[pitrou]

This code isn't new, so it's surprising it's failing "more and more"?

[vstinner]

This code isn't new, so it's surprising it's failing "more and more"?

I modified the CI recently to stop ignoring silently tests failing randomly (FAILURE then SUCCESS): pass new --fail-rerun option to regrtest. It's likely that this hang was silently ignored by CIs previously.

The affected buildbot "PPC64LE Fedora Stable Refleaks 3.x" is blocking Python releases, it's part of STABLE buildbots.

[pitrou]

Well... do you want to undo the change on the failing buildbot until we fix this issue?

[pitrou]

Please take a look at alternate PR #109629

[vstinner]

do you want to undo the change on the failing buildbot until we fix this issue?

Oh sure, if i don't have the bandwith to fix regressions, i will undo this change, once we listed failing tests. That would be reasonable.

[vstinner]

Since @pitrou has a better approach, i convert this change to a draft for now.