Repository navigation
mmap.mmap() should not necessarily clone the file descriptor #78502
Description
| BPO | 34321 |
|---|---|
| Nosy | @tiran, @MojoVampire, @ZackerySpytz, @manuels |
| PRs |
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']
Activity
manuels commentedon Aug 2, 2018
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 commentedon 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 commentedon 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
gh-78502: Add a trackfd parameter to mmap.mmap() (GH-25425)
encukou commentedon 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!
1 remaining item
encukou commentedon 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.
pythongh-78502: Add a trackfd parameter to mmap.mmap() (pythonGH-25425)
pythongh-78502: Add a trackfd parameter to mmap.mmap() (pythonGH-25425)
pythongh-78502: Add a trackfd parameter to mmap.mmap() (pythonGH-25425)
Replace memmap usage with direct file I/O
Replace memmap usage with direct file I/O (#82)
WIP: pythongh-78502: Add support of trackfd in mmap() on Windows
pythongh-78502: AAdd a trackfd parameter to mmap.mmap() on Windows
serhiy-storchaka commentedon Aug 29, 2025
#138238 adds it for Windows.
pythongh-78502: AAdd a trackfd parameter to mmap.mmap() on Windows
gh-78502: Add a trackfd parameter to mmap.mmap() on Windows (GH-138238)
pythongh-78502: Add a trackfd parameter to mmap.mmap() on Windows (py…