For Windows we should ensure that the GIL is released for all calls to CreateFileMapping / MapViewOfFile, UnmapViewOfFile, SetFilePointer / SetEndOfFile, and CloseHandle (for the section and file handle). The CloseHandle calls can actually take as long as CreateFileMapping, MapViewOfFile, and UnmapViewOfFile. But to put this into perspective, a CreateFile call takes about 10 times longer. Note that SetEndOfFile can be on the order of a CreateFile call in terms of I/O system-call overhead and updating filesystem records for allocated disk space, but it doesn't actually zero the extended region on disk, which would be far more expensive. Zeroing occurs on access when the valid data length is extended.

Another issue I notice with this module is that tagname is a char * that we're encoding as UTF-8 and passing to ANSI CreateFileMappingA. This creates mojibake for non-ASCII strings on all but the latest Windows 10 systems that have the system locale configure to use UTF-8. tagname should be a wchar_t *, and we should be calling CreateFileMappingW.

Thank you, Eryk Sun.

If it is possible for access violations to occur when the mmap object is, for example, accessed by multiple threads, the GIL should not be released.

For what it's worth, I did encounter an access violation when releasing the GIL in mmap.resize() at some point (when using threads). It reminded me of https://bugs.python.org/issue16212, though that bug was encountered on Linux.

I will create a BPO issue for the mojibake issue.

Right. Releasing the global lock in mmap_resize_method would require redesigning the mmap module to synchronize access with a local lock, similar to what "_io/bufferedio.c" has to do to protect its internal state. resize isn't called frequently enough to justify the potential for introducing new bugs here.

I did a little timing test in C to gauge the relative time taken by the calls needed to create a file, resize it to 128 MiB, get the size, create a section for it, map a view, unmap the view, and close the section and file handles. The following is the result for 1000 trials, keeping only values within a standard deviation for each call, and normalizing the averages against that of CreateFileMapping:

In new_mmap_object, we're not releasing the GIL for GetFileSize, but it's a relatively inexpensive call. But the cost of MapViewOfFile is on the order of CreateFileMapping. The two should be viewed as a unit. For example, given m_obj->data is initially NULL:

Py_BEGIN_ALLOW_THREADS
m_obj->map_handle = CreateFileMapping(m_obj->file_handle,
                                      NULL,
                                      flProtect,
                                      size_hi,
                                      size_lo,
                                      m_obj->tagname);
if (m_obj->map_handle == NULL) {
    dwErr = GetLastError();
} else {
    m_obj->data = (char *) MapViewOfFile(m_obj->map_handle,
                                         dwDesiredAccess,
                                         off_hi,
                                         off_lo,
                                         m_obj->size);
    if (m_obj->data == NULL) {
        dwErr = GetLastError();
        CloseHandle(m_obj->map_handle);
        m_obj->map_handle = NULL;
    }
}
Py_END_ALLOW_THREADS
if (m_obj->data == NULL) {
    Py_DECREF(m_obj);
    PyErr_SetFromWindowsErr(dwErr);
    return NULL;
}
return (PyObject *)m_obj;

In new_mmap_object, we're not releasing the GIL for GetFileSize, but it's a relatively inexpensive call.

On second thought, the file could be remote, in which case the underlying NtQueryInformationFile: FileStandardInformation call is subject to network latency. We should release the GIL when calling GetFileSize in mmap_size_method and new_mmap_object.

This is unrelated, but a possible bug. For Unix, shouldn't mmap_size_method return PyLong_FromSsize_t(self->size) if self->fd is -1, like the Windows implementation? Currently in Linux size() of an anonymous mapping raises OSError: [Errno 9] Bad file descriptor.

Hey, @isidentical @ZackerySpytz @eryksun: this has been authored and accepted two years back. If this is still relevant, can you rebase on main?

✅ Deploy Preview for python-cpython-preview ready!

Name	Link
🔨 Latest commit	c196466
🔍 Latest deploy log	https://app.netlify.com/sites/python-cpython-preview/deploys/6396a25564a80c0009885b80
😎 Deploy Preview	https://deploy-preview-14114--python-cpython-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

The following commit authors need to sign the Contributor License Agreement:

• 33177550+nanjekyejoannah@users.noreply.github.com