For Windows we should ensure that the GIL is released for all calls toCreateFileMapping /MapViewOfFile,UnmapViewOfFile,SetFilePointer /SetEndOfFile, andCloseHandle (for the section and file handle). TheCloseHandle calls can actually take as long asCreateFileMapping,MapViewOfFile, andUnmapViewOfFile. But to put this into perspective, aCreateFile call takes about 10 times longer. Note thatSetEndOfFile can be on the order of aCreateFile 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 thattagname is achar * that we're encoding as UTF-8 and passing to ANSICreateFileMappingA. 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 awchar_t *, and we should be callingCreateFileMappingW.

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 inmmap.resize() at some point (when using threads). It reminded me ofhttps://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 inmmap_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 ofCreateFileMapping:

Innew_mmap_object, we're not releasing the GIL forGetFileSize, but it's a relatively inexpensive call. But the cost ofMapViewOfFile is on the order ofCreateFileMapping. The two should be viewed as a unit. For example, givenm_obj->data is initiallyNULL:

Py_BEGIN_ALLOW_THREADSm_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_THREADSif (m_obj->data == NULL) {    Py_DECREF(m_obj);    PyErr_SetFromWindowsErr(dwErr);    return NULL;}return (PyObject *)m_obj;

Innew_mmap_object, we're not releasing the GIL forGetFileSize, but it's a relatively inexpensive call.

On second thought, the file could be remote, in which case the underlyingNtQueryInformationFile:FileStandardInformation call is subject to network latency. We should release the GIL when callingGetFileSize inmmap_size_method andnew_mmap_object.

This is unrelated, but a possible bug. For Unix, shouldn'tmmap_size_method returnPyLong_FromSsize_t(self->size) ifself->fd is -1, like the Windows implementation? Currently in Linuxsize() of an anonymous mapping raisesOSError: [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 onmain?

✅ Deploy Preview forpython-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 yourNetlify site settings.

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

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