Issue 31310: semaphore tracker isn't protected against crashes

Similar to bpo-31308, but pertaining to multiprocessing's semaphore tracker process.

New changeset cbe1756e3ecefc0e24a5d0a4b8663db9b6d0cc52 by Antoine Pitrou in branch 'master':
bpo-31310: multiprocessing's semaphore tracker should be launched again if crashed (#3247)
https://github.com/python/cpython/commit/cbe1756e3ecefc0e24a5d0a4b8663db9b6d0cc52

New changeset b5f09acf0a0219cec32b7eba3acdcb573fc74ab5 by Antoine Pitrou in branch '3.6':
[3.6] bpo-31310: multiprocessing's semaphore tracker should be launched again if crashed (GH-3247) (#4254)
https://github.com/python/cpython/commit/b5f09acf0a0219cec32b7eba3acdcb573fc74ab5

With this fix, the semaphore_tracker is not shared with the children anymore and each process launches its own tracker.

I opened a PR to try to fix it. Let me know if I should open a new ticket.

> With this fix, the semaphore_tracker is not shared with the children anymore and each process launches its own tracker.

That's only if the semaphore tracker process crashes, right?

For new processes created with spawn or forkserver, only the semaphore_tracker._fd is send and shared to the child. Thus, as the _pid argument is None in the new process, it launches a new tracker if it needs to create a new Semaphore, regardless of crashes in the previous semaphore tracker.

> Thus, as the _pid argument is None in the new process, it launches a new tracker if it needs to create a new Semaphore

I see.  Is it an optimization problem, or does it risk leaking semaphores?  The problem is if a semaphore shared with the child isn't garbage collected correctly.

> Is it an optimization problem, or does it risk leaking semaphores?

I do not think it risks leaking semaphores as the clean-up is performed by the process which created the Semaphore. So I would say it is more an optimization issue.

It is true that I do not see many use case where the Semaphore might be created by the child process but it was optimized in previous version. If you don't think it is useful, we could avoid sharing the semaphore_tracker pipe to the child process to reduce complexity in the spawning process. Do you think it is worth fixing or it should be simplified?

Tests are failed when ran with -Werror.

$ ./python -We -m test -vuall test_multiprocessing_fork test_multiprocessing_forkserver test_multiprocessing_spawn
...
======================================================================
ERROR: test_semaphore_tracker_sigkill (test.test_multiprocessing_fork.TestSemaphoreTracker)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/_test_multiprocessing.py", line 4582, in test_semaphore_tracker_sigkill
    self.check_semaphore_tracker_death(signal.SIGKILL, True)
  File "/home/serhiy/py/cpython/Lib/test/_test_multiprocessing.py", line 4546, in check_semaphore_tracker_death
    _semaphore_tracker.ensure_running()
  File "/home/serhiy/py/cpython/Lib/multiprocessing/semaphore_tracker.py", line 64, in ensure_running
    warnings.warn('semaphore_tracker: process died unexpectedly, '
UserWarning: semaphore_tracker: process died unexpectedly, relaunching.  Some semaphores might leak.

----------------------------------------------------------------------
...
======================================================================
ERROR: test_semaphore_tracker_sigint (test.test_multiprocessing_forkserver.TestSemaphoreTracker)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/_test_multiprocessing.py", line 4574, in test_semaphore_tracker_sigint
    self.check_semaphore_tracker_death(signal.SIGINT, False)
  File "/home/serhiy/py/cpython/Lib/test/_test_multiprocessing.py", line 4546, in check_semaphore_tracker_death
    _semaphore_tracker.ensure_running()
  File "/home/serhiy/py/cpython/Lib/multiprocessing/semaphore_tracker.py", line 64, in ensure_running
    warnings.warn('semaphore_tracker: process died unexpectedly, '
UserWarning: semaphore_tracker: process died unexpectedly, relaunching.  Some semaphores might leak.

======================================================================
ERROR: test_semaphore_tracker_sigkill (test.test_multiprocessing_forkserver.TestSemaphoreTracker)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/_test_multiprocessing.py", line 4582, in test_semaphore_tracker_sigkill
    self.check_semaphore_tracker_death(signal.SIGKILL, True)
  File "/home/serhiy/py/cpython/Lib/test/_test_multiprocessing.py", line 4554, in check_semaphore_tracker_death
    sem = ctx.Semaphore()
  File "/home/serhiy/py/cpython/Lib/multiprocessing/context.py", line 82, in Semaphore
    return Semaphore(value, ctx=self.get_context())
  File "/home/serhiy/py/cpython/Lib/multiprocessing/synchronize.py", line 126, in __init__
    SemLock.__init__(self, SEMAPHORE, value, SEM_VALUE_MAX, ctx=ctx)
  File "/home/serhiy/py/cpython/Lib/multiprocessing/synchronize.py", line 80, in __init__
    register(self._semlock.name)
  File "/home/serhiy/py/cpython/Lib/multiprocessing/semaphore_tracker.py", line 104, in register
    self._send('REGISTER', name)
  File "/home/serhiy/py/cpython/Lib/multiprocessing/semaphore_tracker.py", line 111, in _send
    self.ensure_running()
  File "/home/serhiy/py/cpython/Lib/multiprocessing/semaphore_tracker.py", line 64, in ensure_running
    warnings.warn('semaphore_tracker: process died unexpectedly, '
UserWarning: semaphore_tracker: process died unexpectedly, relaunching.  Some semaphores might leak.

----------------------------------------------------------------------
...
======================================================================
ERROR: test_semaphore_tracker_sigint (test.test_multiprocessing_spawn.TestSemaphoreTracker)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/_test_multiprocessing.py", line 4574, in test_semaphore_tracker_sigint
    self.check_semaphore_tracker_death(signal.SIGINT, False)
  File "/home/serhiy/py/cpython/Lib/test/_test_multiprocessing.py", line 4546, in check_semaphore_tracker_death
    _semaphore_tracker.ensure_running()
  File "/home/serhiy/py/cpython/Lib/multiprocessing/semaphore_tracker.py", line 64, in ensure_running
    warnings.warn('semaphore_tracker: process died unexpectedly, '
UserWarning: semaphore_tracker: process died unexpectedly, relaunching.  Some semaphores might leak.

======================================================================
ERROR: test_semaphore_tracker_sigkill (test.test_multiprocessing_spawn.TestSemaphoreTracker)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/_test_multiprocessing.py", line 4582, in test_semaphore_tracker_sigkill
    self.check_semaphore_tracker_death(signal.SIGKILL, True)
  File "/home/serhiy/py/cpython/Lib/test/_test_multiprocessing.py", line 4554, in check_semaphore_tracker_death
    sem = ctx.Semaphore()
  File "/home/serhiy/py/cpython/Lib/multiprocessing/context.py", line 82, in Semaphore
    return Semaphore(value, ctx=self.get_context())
  File "/home/serhiy/py/cpython/Lib/multiprocessing/synchronize.py", line 126, in __init__
    SemLock.__init__(self, SEMAPHORE, value, SEM_VALUE_MAX, ctx=ctx)
  File "/home/serhiy/py/cpython/Lib/multiprocessing/synchronize.py", line 80, in __init__
    register(self._semlock.name)
  File "/home/serhiy/py/cpython/Lib/multiprocessing/semaphore_tracker.py", line 104, in register
    self._send('REGISTER', name)
  File "/home/serhiy/py/cpython/Lib/multiprocessing/semaphore_tracker.py", line 111, in _send
    self.ensure_running()
  File "/home/serhiy/py/cpython/Lib/multiprocessing/semaphore_tracker.py", line 64, in ensure_running
    warnings.warn('semaphore_tracker: process died unexpectedly, '
UserWarning: semaphore_tracker: process died unexpectedly, relaunching.  Some semaphores might leak.

----------------------------------------------------------------------

Sorry, that bug was actually introduced in issue33613.

messages: + msg327243
stage: needs patch -> resolved

nosy: + serhiy.storchaka
messages: + msg326827

stage: resolved -> needs patch