This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: multiple problems with mmap.resize() in Windows
Type: behavior Stage: resolved
Components: IO, Library (Lib), Windows Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: tim.golden Nosy List: corona10, eryksun, neonene, paul.moore, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2020-06-08 23:08 by eryksun, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 29213 closed tim.golden, 2021-10-25 15:30
PR 30175 merged neonene, 2021-12-17 16:22
Messages (4)
msg371050 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2020-06-08 23:08
In Windows, mmap.resize() unmaps the view of the section [1], closes the section handle, resizes the file (without error checking), creates a new section (without checking for ERROR_ALREADY_EXISTS), and maps a new view. This code has several problems.

Case 1

If it tries to shrink a file that has existing section references, SetEndOfFile fails with ERROR_USER_MAPPED_FILE. Since the error isn't checked, the code proceeds to silently map a smaller view without changing the file size, which contradicts the documented behavior. For example:

    >>> f = open('C:/Temp/spam.txt', 'wb+')
    >>> f.truncate(8192)
    8192
    >>> m1 = mmap.mmap(f.fileno(), 0)
    >>> m2 = mmap.mmap(f.fileno(), 0)
    >>> m1.resize(4096)
    >>> os.fstat(f.fileno()).st_size
    8192

In this case, it should set dwErrCode = ERROR_USER_MAPPED_FILE; set new_size = self->size; remap the file; and fail the resize() call. The mmap object should remain open.

Case 2

Growing the file may succeed, but if it's a named section (i.e. self->tagname is set) with multiple handle references, then CreateFileMapping just reopens the existing section by name. Or there could be a race condition with another section that gets created with the same name in the window between closing the current section, resizing the file, and creating a new section. Generally mapping a new view will fail in this case. In particular, if the section isn't large enough, then the NtMapViewOfSection system call fails with STATUS_INVALID_VIEW_SIZE, which WinAPI MapViewOfFile translates to ERROR_ACCESS_DENIED. For example:

    >>> f = open('C:/Temp/spam.txt', 'wb+')
    >>> f.truncate(8192)
    8192
    >>> m1 = mmap.mmap(f.fileno(), 0, 'spam')
    >>> m2 = mmap.mmap(-1, 8192, 'spam')
    >>> m1.resize(16384)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    PermissionError: [WinError 5] Access is denied
    >>> m1[0]
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: mmap closed or invalid
   >>> os.fstat(f.fileno()).st_size
    16384

If CreateFileMapping succeeds with the last error set to ERROR_ALREADY_EXISTS, then it either opened the previous section or there was a race condition in which another section was created with the same name. This case should be handled by closing the section and failing the call. It should not proceed to MapViewOfFile, which may 'succeed' unreliably.

Additionally, there could be a LBYL check at the outset to avoid having to close the section in the case of a named section that's opened multiple times. If self->tagname is set, get the handle count of the section via NtQueryObject: ObjectBasicInformation [2]. If the handle count is not exactly 1, fail the resize() call, but don't close the mmap object. 

Case 3

resize() is broken for sections that are backed by the system paging file. For example:

    >>> m = mmap.mmap(-1, 4096)
    >>> m.resize(8192)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    OSError: [WinError 87] The parameter is incorrect
    >>> m[0]
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: mmap closed or invalid

Since the handle value is INVALID_HANDLE_VALUE in this case, the calls to resize the 'file' via SetFilePointer and SetEndOfFile fail with ERROR_INVALID_HANDLE (due to an object type mismatch because handle value -1 is the current process). There's no error checking, so it proceeds to call CreateFileMapping with dwMaximumSizeHigh and dwMaximumSizeLow as 0, which fails immediately as an invalid parameter. Creating a section that's backed by the system paging file requires a non-zero size.

The view of a section can shrink, but the section itself cannot grow. Generally resize() in this case would have to create a copy as follows: close the section; create another section with the new size; map the new view; copy the contents of the old view to the new view; and unmap the old view. Note that creating a new named section may open the previous section, or open an unrelated section if there's a race condition, so ERROR_ALREADY_EXISTS has to be handled.

[1]: "section" is the native system name of a "file mapping".
[2]: https://docs.microsoft.com/en-us/windows/win32/api/winternl/nf-winternl-ntqueryobject
msg371156 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2020-06-10 02:48
For a concrete example, here's a rewrite of the Windows implementation that incorporates the suggested changes.

#include <winternl.h>

#ifdef MS_WINDOWS
        /* a named file mapping that's open more than once can't be resized */
        /* this check could be moved into is_resizeable */
        if (self->tagname) {
            typedef NTSTATUS NTAPI ntqo_f(HANDLE, OBJECT_INFORMATION_CLASS,
                PVOID, ULONG, PULONG);
            ntqo_f *pNtQueryObject = (ntqo_f *)GetProcAddress(
                GetModuleHandleW(L"ntdll"), "NtQueryObject");
            if (pNtQueryObject) {
                PUBLIC_OBJECT_BASIC_INFORMATION info;
                NTSTATUS status = pNtQueryObject(self->map_handle,
                    ObjectBasicInformation, &info, sizeof(info), NULL);
                if (NT_SUCCESS(status) && info.HandleCount > 1) {
                    PyErr_SetFromWindowsErr(ERROR_USER_MAPPED_FILE);
                    return NULL;
                }
            }
        }
        DWORD error = 0;
        char *old_data = self->data;
        LARGE_INTEGER offset, max_size;
        offset.QuadPart = self->offset;
        max_size.QuadPart = new_size;
        /* close the file mapping */
        CloseHandle(self->map_handle);
        self->map_handle = NULL;
        /* if it's not the paging file, unmap the view and resize the file */
        if (self->file_handle != INVALID_HANDLE_VALUE) {
            UnmapViewOfFile(self->data);
            self->data = NULL;
            /* resize the file */
            if (!SetFilePointerEx(self->file_handle, max_size, NULL,
                     FILE_BEGIN) ||
                !SetEndOfFile(self->file_handle)) {
                /* resizing failed. try to remap the file */
                error = GetLastError();
                new_size = max_size.QuadPart = self->size;
            }
        }
        /* create a new file mapping and map a new view */
        /* FIXME: call CreateFileMappingW with wchar_t tagname */
        self->map_handle = CreateFileMapping(self->file_handle, NULL,
            PAGE_READWRITE, max_size.HighPart, max_size.LowPart,
            self->tagname);
        if (self->map_handle != NULL &&
              GetLastError() != ERROR_ALREADY_EXISTS) {
            self->data = MapViewOfFile(self->map_handle, FILE_MAP_WRITE,
                offset.HighPart, offset.LowPart, new_size);
            if (self->data != NULL) {
                /* copy the old view if using the paging file */
                if (self->file_handle == INVALID_HANDLE_VALUE) {
                    memcpy(self->data, old_data,
                        self->size < new_size ? self->size : new_size);
                }
                self->size = new_size;
            } else {
                error = GetLastError();
                CloseHandle(self->map_handle);
                self->map_handle = NULL;
            }
        } else {
            error = GetLastError();
        }
        /* unmap the old view if using the paging file */
        if (self->file_handle == INVALID_HANDLE_VALUE) {
            UnmapViewOfFile(old_data);
        }
        if (error) {
            PyErr_SetFromWindowsErr(error);
            return NULL;
        }
        Py_RETURN_NONE;
#endif /* MS_WINDOWS */
msg405059 - (view) Author: Tim Golden (tim.golden) * (Python committer) Date: 2021-10-26 21:56
New changeset aea5ecc458084e01534ea6a11f4181f369869082 by Tim Golden in branch 'main':
bpo-40915: Fix mmap resize bugs on Windows (GH-29213)
https://github.com/python/cpython/commit/aea5ecc458084e01534ea6a11f4181f369869082
msg408844 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-12-18 13:03
New changeset 6214caafbe66e34e84c1809abf0b7aab6791956b by neonene in branch 'main':
bpo-40915: Avoid compiler warnings by fixing mmapmodule conversion from LARGE_INTEGER to Py_ssize_t (GH-30175)
https://github.com/python/cpython/commit/6214caafbe66e34e84c1809abf0b7aab6791956b
History
Date User Action Args
2022-04-11 14:59:32adminsetgithub: 85092
2021-12-18 13:03:46steve.dowersetmessages: + msg408844
2021-12-17 16:22:50neonenesetnosy: + neonene

pull_requests: + pull_request28391
2021-10-26 21:57:56tim.goldensetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-10-26 21:56:53tim.goldensetmessages: + msg405059
2021-10-25 15:30:10tim.goldensetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request27477
2021-10-23 13:53:02tim.goldenlinkissue2733 superseder
2021-10-23 07:19:12tim.goldensetassignee: tim.golden
2020-09-19 00:55:02corona10setnosy: + corona10
2020-06-10 02:48:16eryksunsetmessages: + msg371156
2020-06-10 02:42:46eryksunsetmessages: - msg371153
2020-06-10 02:26:33eryksunsetmessages: + msg371153
2020-06-08 23:08:49eryksunsettitle: muultiple problems with mmap.resize() in Windows -> multiple problems with mmap.resize() in Windows
2020-06-08 23:08:03eryksuncreate