GeezerVison is showing github.com — mmap.mmap() should not necessarily clone the file descriptor · Issue #78502 · python/cpython
mmap.mmap() should not necessarily clone the file descriptor #78502Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2018-08-02.13:22:03.631>
labels = ['type-feature', 'library', '3.10']
title = 'mmap.mmap() should not necessarily clone the file descriptor'
updated_at = <Date 2021-04-15.16:45:08.950>
user = 'https://github.com/manuels' bugs.python.org fields:
activity = <Date 2021-04-15.16:45:08.950>
actor = 'ZackerySpytz'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2018-08-02.13:22:03.631>
creator = 'manuels'
dependencies = []
files = []
hgrepos = []
issue_num = 34321
keywords = ['patch']
message_count = 3.0
messages = ['322953', '323069', '386013']
nosy_count = 4.0
nosy_names = ['christian.heimes', 'josh.r', 'ZackerySpytz', 'manuels']
pr_nums = ['25425']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue34321'
versions = ['Python 3.10'] Linked PRs
Activitymanuels commented on Aug 2, 2018 MannequinAuthor More actions mmap.mmap(fileno, length, flags, prot, access, offset) always clones the file descriptor that should be used [1].
The cloning of the file descriptor seems to be done to ensure that the file cannot be closed behind mmap's back, but if you are mmap()'ing a lot of memory regions of a file this can cause a 'Too many open files' error.
I would suggest to add an option to mmap.mmap() that tells it not to clone the file descriptor. This can cause an issue if the file is closed before accessing the mmapped region, so this fact should also be pointed out in the documentation.
[1] https://github.com/python/cpython/blob/master/Modules/mmapmodule.c#L1159 MojoVampire commented on Aug 3, 2018 Why would it "cause an issue if the file is closed before accessing the mmapped region"? As shown in your own link, the constructor performs the mmap call immediately after the descriptor is duplicated, with the GIL held; any race condition that could close the file before the mmap occurs could equally well close it before the descriptor is duplicated.
The possible issues aren't tied to accessing the memory (once the mapping has been performed, the file descriptor can be safely closed in general), but rather, to the size and resize methods of mmap objects (the former using the fd to fstat the file, the latter using it to ftruncate the file). As long as you don't use size/resize, nothing else depends on the file descriptor after construction has completed. The size method in particular seems like a strange wart on the API; it returns the total file size, not the size of the mapping (len(mapping) gets the size of the actual mapping). tiran commented on Jan 31, 2021 This issue came up in another discussion. I have given it some thought. mmap.mmap() dups the FD because its close() and __exit__() methods close(2) the fd. The size() and resize() methods use the fd to determine the size of the underlying file or to resize the file.
The easiest way to solve the issue while avoind footguns, is an option to not track the fd at all, e.g. "trackfd" with default "True". mmap(..., trackfd=False) would neither dup the fd nor store the fd in its internal struct. In untracked fd case, size() and resize() would no longer work. That's totally fine for mappings without PROT_WRITE.
It's safe to close the fd after mmap call, see https://man7.org/linux/man-pages/man2/mmap.2.html
After the mmap() call has returned, the file descriptor, fd, can
be closed immediately without invalidating the mapping.
encukou commented on Jan 16, 2024 Thank you manuels for the report, Chrisitian for the proposal, Zackery for the initial PR, and all the reviewers for the polish! encukou commented on Jan 16, 2024 Oh, I suppose I shouldn't close this yet: it would be good to add it for Windows too.
But Windows code is separate, and I don't have a Windows box set up yet. serhiy-storchaka commented on Aug 29, 2025
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|