semaphore tracker isn't protected against crashes #75491

pitrou · 2017-08-30T15:52:33Z

BPO	31310
Nosy	@pitrou, @serhiy-storchaka, @applio, @tomMoral, @pablogsal
PRs	bpo-31310: multiprocessing's semaphore tracker should be launched again if crashed #3247 [3.6] bpo-31310: multiprocessing's semaphore tracker should be launched again if crashed (GH-3247) #4254 bpo-36668: FIX reuse semaphore tracker for child processes #5172

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-10-06.13:15:03.035>
created_at = <Date 2017-08-30.15:52:32.836>
labels = ['3.7', 'type-bug', 'library']
title = "semaphore tracker isn't protected against crashes"
updated_at = <Date 2018-10-06.13:15:03.034>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2018-10-06.13:15:03.034>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2018-10-06.13:15:03.035>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2017-08-30.15:52:32.836>
creator = 'pitrou'
dependencies = []
files = []
hgrepos = []
issue_num = 31310
keywords = ['patch']
message_count = 10.0
messages = ['301029', '305480', '305485', '309888', '309889', '309890', '309893', '309895', '326827', '327243']
nosy_count = 5.0
nosy_names = ['pitrou', 'serhiy.storchaka', 'davin', 'tomMoral', 'pablogsal']
pr_nums = ['3247', '4254', '5172']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue31310'
versions = ['Python 3.6', 'Python 3.7']

pitrou · 2017-08-30T15:52:33Z

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

pitrou · 2017-11-03T13:31:40Z

New changeset cbe1756 by Antoine Pitrou in branch 'master':
bpo-31310: multiprocessing's semaphore tracker should be launched again if crashed (bpo-3247)
cbe1756

pitrou · 2017-11-03T13:58:40Z

New changeset b5f09ac by Antoine Pitrou in branch '3.6':
[3.6] bpo-31310: multiprocessing's semaphore tracker should be launched again if crashed (GH-3247) (bpo-4254)
b5f09ac

tomMoral · 2018-01-13T10:39:41Z

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.

pitrou · 2018-01-13T15:36:16Z

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?

tomMoral · 2018-01-13T15:51:15Z

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.

pitrou · 2018-01-13T17:41:34Z

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.

tomMoral · 2018-01-13T18:06:23Z

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?

serhiy-storchaka · 2018-10-01T20:43:11Z

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.

serhiy-storchaka · 2018-10-06T13:15:03Z

Sorry, that bug was actually introduced in bpo-33613.

pitrou added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Aug 30, 2017

pitrou closed this as completed Nov 3, 2017

serhiy-storchaka reopened this Oct 1, 2018

serhiy-storchaka closed this as completed Oct 6, 2018

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

semaphore tracker isn't protected against crashes #75491

semaphore tracker isn't protected against crashes #75491

pitrou commented Aug 30, 2017

pitrou commented Aug 30, 2017

pitrou commented Nov 3, 2017

pitrou commented Nov 3, 2017

tomMoral mannequin commented Jan 13, 2018

pitrou commented Jan 13, 2018

tomMoral mannequin commented Jan 13, 2018

pitrou commented Jan 13, 2018

tomMoral mannequin commented Jan 13, 2018

serhiy-storchaka commented Oct 1, 2018

serhiy-storchaka commented Oct 6, 2018

semaphore tracker isn't protected against crashes #75491

semaphore tracker isn't protected against crashes #75491

Comments

pitrou commented Aug 30, 2017

pitrou commented Aug 30, 2017

pitrou commented Nov 3, 2017

pitrou commented Nov 3, 2017

tomMoral mannequin commented Jan 13, 2018

pitrou commented Jan 13, 2018

tomMoral mannequin commented Jan 13, 2018

pitrou commented Jan 13, 2018

tomMoral mannequin commented Jan 13, 2018

serhiy-storchaka commented Oct 1, 2018

serhiy-storchaka commented Oct 6, 2018