Skip to content

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

Merged
pitrou merged 7 commits into
python:masterfrom
pierreglaser:silence-ressource-tracker-with-manager-warning
May 13, 2019
Merged

bpo-36867: Create the resource_tracker before launching SharedMemoryManagers#13276
pitrou merged 7 commits into
python:masterfrom
pierreglaser:silence-ressource-tracker-with-manager-warning

Conversation

@pierreglaser

@pierreglaserpierreglaser commented May 13, 2019

Copy link
Copy Markdown
Contributor

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:

frommultiprocessing.managersimportSharedMemoryManagerfrommultiprocessingimportresource_trackersmm=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: Thereappeartobe1leakedshared_memoryobjectstocleanupatshutdownwarnings.warn('resource_tracker: There appear to be %d '/home/pierreglaser/repos/cpython/Lib/multiprocessing/resource_tracker.py:211: UserWarning: resource_tracker: '/psm_3bfa242d': [Errno2] Nosuchfileordirectory: '/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 unlinkssl. 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
pierreglaserforce-pushed the silence-ressource-tracker-with-manager-warning branch from f48bd3b to 0c904b5CompareMay 13, 2019 12:54
@pierreglaserpierreglaser changed the title bpo-36867: bpo-36867: Create the resource_tracker before launching SharedMemoryManagersMay 13, 2019

@auvipyauvipy left a comment

Copy link
Copy Markdown

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
Copy Markdown
ContributorAuthor

@pitrou how does this look to you?

Comment threadLib/multiprocessing/managers.py
@pitrou

Copy link
Copy Markdown
Member

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
Copy Markdown
ContributorAuthor

@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
Copy Markdown
Member

I see. Is there the same problem with semaphores?

@pierreglaser

Copy link
Copy Markdown
ContributorAuthor

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
Copy Markdown
Member

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

@pitrou

Copy link
Copy Markdown
Member

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

@pierreglaser

Copy link
Copy Markdown
ContributorAuthor

OK I'll make a docfix PR.

@pitrou

Copy link
Copy Markdown
Member

Ok, I'll merge this now.

@pitrou

pitrou commented May 13, 2019

Copy link
Copy Markdown
Member

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

pitrou commented May 13, 2019

Copy link
Copy Markdown
Member

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.

Comment threadLib/test/_test_multiprocessing.py Outdated
@pierreglaser

Copy link
Copy Markdown
ContributorAuthor

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

@pitrou
pitrouforce-pushed the silence-ressource-tracker-with-manager-warning branch from 3e6ec27 to 4ef01c8CompareMay 13, 2019 18:00
@pitrou

Copy link
Copy Markdown
Member

@pierreglaser You probably needed -Wd.

@pierreglaser

Copy link
Copy Markdown
ContributorAuthor

Looking good @pitrou :)

@pitrou
pitrou merged commit b1dfcad into python:masterMay 13, 2019
@pitrou

Copy link
Copy Markdown
Member

Looks like we can move on :-)

Sign up for freeto 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

@pierreglaser@pitrou@auvipy@the-knights-who-say-ni@bedevere-bot