Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-36977: Make SharedMemoryManager release its resources if its parent process dies#13451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
fc7126b87c889f17998976e5d7eddd6e558File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -3824,6 +3824,45 @@ def test_shared_memory_SharedMemoryServer_ignores_sigint(self): | ||
| smm.shutdown() | ||
| def test_shared_memory_SharedMemoryManager_cleanup_if_parent_dies(self): | ||
| # If a process that spawned a SharedMemoryManager was terminated, its | ||
| # manager process should notice it, unlink the resources the manager | ||
| # delivered, and shut down. | ||
| cmd = '''if 1: | ||
| import sys, time | ||
| from multiprocessing.managers import SharedMemoryManager | ||
| smm = SharedMemoryManager() | ||
| smm.start() | ||
| sl = smm.ShareableList([1, 2, 3]) | ||
| sys.stdout.write(f'{sl.shm.name}\\n') | ||
| sys.stdout.flush() | ||
| time.sleep(100) | ||
| ''' | ||
| p = subprocess.Popen([sys.executable, '-E', '-c', cmd], | ||
| stdout=subprocess.PIPE, stderr=subprocess.PIPE) | ||
| shm_name = p.stdout.readline().decode().strip() | ||
| p.terminate() | ||
| start_time = time.monotonic() | ||
| deadline = start_time + 60 | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 10 seconds sounds enough, no? Contributor
| ||
| t = 0.1 | ||
| while time.monotonic() < deadline: | ||
| time.sleep(t) | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't feel comfortable having tests that rely on sleep, they are almost assured to fail in some of the buildbots ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand your worries. I am having trouble thinking of a test that does not require a timeout though (else, we risk hanging forever waiting for the synchronization event to occur). Inherently, this tests a feature that relies on Note that using this pattern: whiletime.monotonic() <deadline: time.sleep(t) ifcondition(): breakelse: raiseAssertionErroris already more robust than a simple time.sleep(dt) self.assertTrue(condition())It was discussed here: https://bugs.python.org/issue36867, and @vstinner seemed OK with its usage. I may be missing something though. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
| ||
| # The shared_memory segment should be released by the | ||
| # SharedMemoryManager server process before it shuts down. | ||
| try: | ||
| sl = shared_memory.SharedMemory(shm_name, create=False) | ||
| except FileNotFoundError: | ||
| break | ||
pierreglaser marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| else: | ||
| raise AssertionError( | ||
| "A SharedMemoryManager prevented some resources to be cleaned " | ||
| "up after its parent process was terminated") | ||
| @unittest.skipIf(os.name != "posix", "resource_tracker is posix only") | ||
| def test_shared_memory_SharedMemoryManager_reuses_resource_tracker(self): | ||
| # bpo-36867: test that a SharedMemoryManager uses the | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| SharedMemoryManager server processes do not block anymore the release of shared memory segments if their parent process was unexpectedly terminated. |
Uh oh!
There was an error while loading. Please reload this page.