gh-155003: Don't keep file descriptor in multiprocessing SharedMemory object

[takluyver]

Once we've created the mmap, we can close the file descriptor immediately, instead of keeping it around in the SharedMemory object.

Without a file descriptor, we can also get rid of the __del__ method, and let the mmap & memoryview finalizers deal with cleanup. This allows shmem.buf to remain usable without having to keep a reference to the SharedMemory object.

• Issue: multiprocessing SharedMemory.__del__ can fail, leak file descriptor #155003

[gpshead]

Going back and forth on this PR with Claude (Fable 5), things we should consider:

grow-in-place resize hacks. SharedMemory has no resize API, so recipes in the wild do os.ftruncate(shm._fd, newsize) — sometimes paired with os.fstat(shm._fd) in the other process to detect the new size before re-mmapping. A few also pass shm._fd over Unix sockets with SCM_RIGHTS. All of that dies with this PR.

Those are "abusing" a private-api via ._fd access, but primarily because they had no other choice. We should attempt to provide them something... which turns out to be for multiple reasons (letting Claude do the digging):

Real _fd uses in the wild:

1. sglang — os.posix_fallocate(shm._fd, 0, nbytes) at line 1924 to avoid SIGBUS on /dev/shm exhaustion:
   https://github.com/sgl-project/sglang/blob/9dcaf6bfdff89b4b29611725ef44161be4e429dd/python/sglang/srt/managers/mm_utils.py#L1919-L1924
2. autonomi-ai/nos — os.chown/os.chmod on shm._fd (lines 45–51) for cross-user permission fixup:
   https://github.com/autonomi-ai/nos/blob/2761f7b50fa3173c74ec63a3321527fbb980b9ac/nos/common/shm.py
3. KolinGuo/RealRobot — reader/writer fcntl locks on shm._fd (lines 486–487) and the ftruncate-resize plan (line 755):
   https://github.com/KolinGuo/RealRobot/blob/42cb3a12c5e0b98023465d54535b3e8faa3692ef/real_robot/utils/multiprocessing/shared_object.py

Related CPython issue:

4. gh-114390 — "SharedMemory crashes on linux with SIGBUS on insufficient shm" (the fallocate-on-create fix):

... me:

The first one for sglang I think could be addressed in this PR as a bugfix: Fix gh-114390 by calling os.posix_fallocate(fd, 0, size) on create (where the platform supports it). This eliminates sglang's need entirely, and it composes with this PR since it happens in __init__ before the fd is closed. Arguably a bugfix and potentially backportable (at least to 3.15). sglang is very widely used so avoiding breaking it by coordinating on this would be good.

But adding support for a mode= parameter like nos wants or public APIs for what the RealRobot code wants to do would be squarely features (3.16+).

... digging in on the things passing the ._fd over sockets. Claude:

1. ChrisJR035/Talos-O-Architecture — the textbook case. It allocates a SharedMemory, then sends shm._fd to a C++ daemon over an AF_UNIX socket for zero-copy tensor handoff:

def send_fd(sock, fd):
    ancillary_data = [(socket.SOL_SOCKET, socket.SCM_RIGHTS, array.array("i", [fd]))]
    sock.sendmsg([b"1"], ancillary_data)
...
send_fd(client, shm._fd)

1. https://github.com/ChrisJR035/Talos-O-Architecture/blob/1d90b5e6ae322279efc5ee65cd2c1b1cfdbd58bf/cognitive_plane/cortex/ignite_native_ipc.py

2. sujaldev/pywaygui — a Wayland client using SharedMemory as its pixel buffer pool. It hands shm._fd to the compositor via conn.flush([shm._fd]); Wayland's wire protocol transmits fds over the unix socket with SCM_RIGHTS (that's the only way wl_shm works). It also creates a second mmap directly from shm._fd with explicit PROT_READ|PROT_WRITE, MAP_SHARED:

3. https://github.com/sujaldev/pywaygui/blob/11c355971886bddd938e47c7c9bfd9fc4230bf24/waygui/client.py

Notable: the fd-passing cases are ones where the held fd matters most, because SCM_RIGHTS needs a live fd at send time. Under PR 155007 these users would have to reopen by name — which works, but ...

... me: is another point in favor of a (beyond this PR) public shared memory opening API to be used for that.

In terms of what to change for this, could we at least do the posix_fallocate and sglang coordination? a sentinel for sglang to use is the existence of the _fd attribute. i've found other code at least acknowledging it being private does not guarantee it exists doing that. sglang today just checks "linux" and assumes. (ugh)

[gpshead]

should we add trackfd=False here? BUT see my other overall comment.

[bedevere-app]

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[takluyver]

Thanks! Given these various uses for the file descriptor, would you prefer me to return to my original suggestion in #155003, keeping the fd around and closing it in __del__? I could also add a .fileno() method to give public access to it.

grow-in-place resize hacks

These could alternatively be handled via mmap, if we leave trackfd=True, either wrapping its size/resize methods or exposing the mmap object as public API. I think the other use cases mentioned do require an fd, though.

...could be addressed in this PR as a bugfix: Fix #114390 by calling os.posix_fallocate(fd, 0, size) on create (where the platform supports it).

I'm happy to integrate that in this PR if you'd like, but I'm not sure it's a straightforward bugfix. With the current behaviour, you can set up a large shared memory area and then use a small part of it, and code doing that might fail if it tries to claim the entire size up front. A similar problem occurs with regular, private memory - you can overallocate and get OOM-killed if you actually try to use all the allocated space.

Maybe leaving a sizeable part of the allocation unused is less common for shared memory? But I'd be cautious of assuming code will always use all the memory space.

another point in favor of a (beyond this PR) public shared memory opening API to be used for that.

Yes, I'd like to see the low-level shm_open & shm_unlink functions publicly accessible & documented alongside the higher-level API. 👍 Python exposes a bunch of other low-level functions for things like timerfds, so it feels odd that these are private. Is there already an issue about this?

[takluyver]

08584a7 adds a reserve=True option to use posix_fallocate if available, defaulting to False. I can switch the default, or even do it without an option if you want.

[takluyver]

I have made the requested changes; please review again

(just read the bot comment)

[bedevere-app]

Thanks for making the requested changes!

@gpshead: please review the changes made to this pull request.

[gpshead]

given things are relyong in ._fd existing I don't know that we'd ever be able to backport even the original change here to release branches. it's a bugfix, but it also breaks behavior things were depending on w.r.t. the fd so being cautious instead of surprising those projects is better. I like the reserve=False feature approach for the gh-114390 fix. Agreed it'd be disruptive to someone's existing setup without the ability to control if fallocate happens or not.

I'm wondering if we need to provide an official way to keep and obtain the fd for the existing (ab)users of ._fd who actually need the fd to use on 3.16? i dislike giant walls of per feature keyword arguments... but a retain_fd= kwarg along with a new .take_ownership_of_retained_fd() method [probably not right, see below] which would raise unless retain_fd=True. and a mmap property to return ._mmap. along with some documentation explaining which people should use for what purposes.

retain_fd=False would also be fine to imply that the mmap(...) can use trackfd=False.

either way though, a decision on what to do when the fd is retained destructor wise seems important. thus the take_ownership fd method naming... if that was called we could track that and not have a destructor os.close() it as it'd become the callers responsibility? or do we need to retain a destructor no matter what when retain_fd=True?

ugh. what a mess. I don't like having a __del__ destructor at all, but either way it could be simplified to not call the big self.close() (the real problem) and just handle the fd closing if present and if we make that conditional, if necessary. (via a separate ._close_fd() method both could call?)

conditionals make it hard to use correctly. so simplifying that whacky idea back to a .fd property instead of any ownership transfer method probably makes more sense. people who get the fd and need it to live beyond our object's close or lifetime should dup it.

(thinking out loud here)

[takluyver]

I've opened #155070 as an alternative to this, keeping the fd and fixing __del__.

I think adding a retain_fd option is unnecessary complexity - it doesn't avoid the need for a __del__ method, and if we're concerned about the extra fd then it's easier to get rid of the duplicate in mmap with trackfd=False. If people want finer control, then I think exposing the shm_open & shm_unlink functions is better than complicating the object-oriented interface.

a new .take_ownership_of_retained_fd() method

I could see something like that being useful, but I think it would need to be more general than SharedMemory. Any object which owns an fd and ensures it gets closed could have an API to disown/detach the fd, leaving it usable and allowing it to outlive the wrapper object. But this is obviously a much bigger scope than what I'm trying to fix here.

I have actually written something related - a class wrapping a received fd which can pass ownership to a file object, a socket object, or hand the caller a raw fd along with the responsibility to close it.