Skip to content

bpo-36867: Create the resource_tracker before launching SharedMemoryManagers #13276

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

Conversation

pierreglaser
Copy link
Contributor

@pierreglaser pierreglaser commented May 13, 2019

TLDR; The resource_tracker needs to be created before SharedMemoryManager processes are launched (which is not the case for managers created using fork)

Take this example below:

from multiprocessing.managers import SharedMemoryManager
from multiprocessing import resource_tracker


smm = SharedMemoryManager()
smm.start()
sl = smm.ShareableList(range(10))
smm.shutdown()

This simple and legitimate python scripts results in a very noisy output:

/home/pierreglaser/repos/cpython/Lib/multiprocessing/resource_tracker.py:198: UserWarning: resource_tracker: There appear to be 1 leaked shared_memory objects to clean up at shutdown
  warnings.warn('resource_tracker: There appear to be %d '
/home/pierreglaser/repos/cpython/Lib/multiprocessing/resource_tracker.py:211: UserWarning: resource_tracker: '/psm_3bfa242d': [Errno 2] No such file or directory: '/psm_3bfa242d'
  warnings.warn('resource_tracker: %r: %s' % (name, e))

Here is why it happens:

  1. smm.start() launches a new manager process. At this point in the main process, no resource_tracker is created if the manager is launched using fork.

  2. sl = smm.ShareableList(range(10)) does two things:

    • First, it creates a new shared_memory object from the parent process. At creation, a resource_tracker.register call is done -> resource_tracker.ensure_running is done -> a new resource_tracker process is created. In addition, this resource_tracker now tracks sl. However, since the manager process was created before, it does not know that a new resource_tracker process was just created in the parent process..
    • Once sl is created, the SharedMemoryManager asks its SharedMemoryTracker ( which lives in the manager process) to track it.
  3. smm.shutdown shuts down the manager. It thus asks the SharedMemoryTracker to destroy sl, which it tracks. To do so,

    • it first opens it (using a SharedMemory(name)) call, which itself triggers a resource_tracker.register call -> resource_tracker.ensure_running call in the manager process, where no resource_tracker exists yet! Thus, another ResourceTracker instance /process is created and starts tracking sl
    • immediatly after, it unlinks sl. This sends an unregister call to the newly created resource_tracker.

When the script ends however, nothing has been done to tell the original resource_tracker created in the parent process that sl was properly unlinked. The resource_tracker thus thinks a leak happened, and tells the user (first line of my output)). However, there was no leak, as sl was properly unlinked by the manager. Therefore, the cleanup attempt of the resource_tracker fails, resulting the second error.

The solution to this problem is simply to create the right before the manager process is created.

https://bugs.python.org/issue36867

@pierreglaser pierreglaser force-pushed the silence-ressource-tracker-with-manager-warning branch from f48bd3b to 0c904b5 Compare May 13, 2019 12:54
@pierreglaser pierreglaser changed the title bpo-36867: bpo-36867: Create the resource_tracker before launching SharedMemoryManagers May 13, 2019
Copy link

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no test?

@pierreglaser
Copy link
Contributor Author

@pitrou how does this look to you?

@pitrou
Copy link
Member

pitrou commented May 13, 2019

I have a more general question: what if the SharedMemoryManager is created in an unrelated process, and then connected to using address and authkey?

@pierreglaser
Copy link
Contributor Author

@pitrou then most likely the process connecting to the SharedMemoryManager and the ResourceTracker will not have the same resource_tracker processes. Similar problems than the one this PR tries to solve would occur.

@pitrou
Copy link
Member

pitrou commented May 13, 2019

I see. Is there the same problem with semaphores?

@pierreglaser
Copy link
Contributor Author

A quick skim through SyncManager, it seems that no, semaphores will not show such problems, because the SyncManager API returns a proxy objcet and not a Semaphore object to the parent process. As no actual semaphore object is created in the parent process, the parent process will not ask its resource_tracker to the object.

@pitrou
Copy link
Member

pitrou commented May 13, 2019

I see. I guess we'll have to live with the limitation.

@pitrou
Copy link
Member

pitrou commented May 13, 2019

By the way, I noticed that the multiprocessing docs still mention the "semaphore tracker process". This should be fixed.

@pierreglaser
Copy link
Contributor Author

OK I'll make a docfix PR.

@pitrou
Copy link
Member

pitrou commented May 13, 2019

Ok, I'll merge this now.

@pitrou
Copy link
Member

pitrou commented May 13, 2019

Hmm, it seems there are still resource tracker warnings on CI (see e.g. Travis builds). Are they expected?

Edit: the warning is from test_shared_memory_cleaned_after_process_termination. So it's expected (and I don't think there's an easy way to silence it).

@pitrou
Copy link
Member

pitrou commented May 13, 2019

There's another ResourceWarning:

test_shared_memory_SharedMemoryManager_reuses_resource_tracker (test.test_multiprocessing_fork.WithProcessesTestSharedMemory) ... /home/antoine/cpython/default/Lib/unittest/case.py:683: ResourceWarning: unclosed file <_io.BufferedReader name=3>
  testMethod()

Edit: will fix it myself.

@pierreglaser
Copy link
Contributor Author

@pitrou I could not reproduce the second ResourceWarning locally...

@pitrou pitrou force-pushed the silence-ressource-tracker-with-manager-warning branch from 3e6ec27 to 4ef01c8 Compare May 13, 2019 18:00
@pitrou
Copy link
Member

pitrou commented May 13, 2019

@pierreglaser You probably needed -Wd.

@pierreglaser
Copy link
Contributor Author

Looking good @pitrou :)

@pitrou pitrou merged commit b1dfcad into python:master May 13, 2019
@pitrou
Copy link
Member

pitrou commented May 13, 2019

Looks like we can move on :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants