-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-82300: Add track parameter to shared memory #110778
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
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Whoops I messed up the documentation part. The tracking part should be on unlink, not on close. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. This looks reasonable, I have mostly questions about the phrasing of the docs.
Do I understand correctly that the whole tracking mechanism only applies on POSIX? Maybe the docs should mention that?
Misc/NEWS.d/next/Library/2023-10-12-18-19-47.gh-issue-82300.P8-O38.rst
Outdated
Show resolved
Hide resolved
@serhiy-storchaka Do you have any experience with this code? Could you help me with the review? |
The new parameter needs tests. |
No, I have no experience with this code. I can help with technical review, but I can't say whether the proposed feature is the right way to solve the problem. It looks like If in most cases |
Thanks, @serhiy-storchaka -- let's see what the PR author says. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @pan324 for submitting this! The proposal looks good on the principle, but please make sure you adress the review comments (including the request for additional unit tests).
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Thanks everyone! Yes, the tracking only happens on POSIX because Windows has its own reference counting. I was cautious about mentioning this because users shouldn't rely on such internal behavior. But I can see that Windows users might that need it as a word of caution to thoroughly test their shared memory deallocation on other systems. Outside of multiprocessing, each Python process comes with its own resource tracker which (assuming Surely the resource tracker itself would need some deeper changes to be more robust but it's not my area of expertise either which is why I have opted for these minimal changes. Windows must not raise an exception for either As far as I understand there is a clash between multiprocessing and subprocess. Users of subprocess always want to disable tracking when not creating. For multiprocessing I use the higher level constructs and therefore never had the need to access the SharedMemory module directly. So while users of SharedMemory typically want I will look into adding suitable tests now. |
…-O38.rst Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
@pitrou Can you complete this review? I really don't know enough about this topic to review it properly. |
Misc/NEWS.d/next/Library/2023-10-12-18-19-47.gh-issue-82300.P8-O38.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Gregory P. Smith <greg@krypto.org>
…ython#110778) Add a track parameter to shared memory to allow resource tracking via the side-launched resource tracker process to be disabled on platforms that use it (POSIX). This allows people who do not want automated cleanup at process exit because they are using the shared memory with processes not participating in Python's resource tracking to use the shared_memory API. Co-authored-by: Łukasz Langa <lukasz@langa.pl> Co-authored-by: Guido van Rossum <gvanrossum@gmail.com> Co-authored-by: Antoine Pitrou <pitrou@free.fr> Co-authored-by: Gregory P. Smith <greg@krypto.org>
…ython#110778) Add a track parameter to shared memory to allow resource tracking via the side-launched resource tracker process to be disabled on platforms that use it (POSIX). This allows people who do not want automated cleanup at process exit because they are using the shared memory with processes not participating in Python's resource tracking to use the shared_memory API. Co-authored-by: Łukasz Langa <lukasz@langa.pl> Co-authored-by: Guido van Rossum <gvanrossum@gmail.com> Co-authored-by: Antoine Pitrou <pitrou@free.fr> Co-authored-by: Gregory P. Smith <greg@krypto.org>
📚 Documentation preview 📚: https://cpython-previews--110778.org.readthedocs.build/