bpo-40915: Fix mmap resize bugs on Windows (GH-29213) #29213
Conversation
…no handle can be resized
… recreated along with its data)
* Check that a resize against the pagefile succeeds and copies as much data as will fit * Check that a resize against a named file correctly fails where another mapping is still open
… an exception but retains the mapping Remove the specific pre-check on resizing a named section but specifically account for it later when resizing
tjguk
changed the title
zooba
left a comment
zooba
left a comment
There was a problem hiding this comment.
Nice change. I still have a couple more questions, but if you're happy with your answers, don't wait on me before merging.
| """ | ||
| start_size = 2 * PAGESIZE | ||
| reduced_size = PAGESIZE | ||
| tagname = "TEST" |
There was a problem hiding this comment.
I'm sorry to have missed this before you merged. Naming kernel objects with generic names like "foo" and "TEST" is a bad idea because the namespace is shared by every process in the current session. It's a serious problem in all of the tests that use tagname. If the name is an existing Section object (i.e. file mapping), CreateFileMapping() will succeed with the last error set to ERROR_ALREADY_EXISTS. In many cases, the Section object likely refers to an unrelated file, but we can't avoid this, or even know about the problem, because mmap doesn't currently support exclusive ("x") creation. This and a few other cases really need to be addressed in the constructor, but that's a separate issue.
There was a problem hiding this comment.
Thanks @eryksun -- good catch. Funnily enough, I'm normally a little over-obsessive with test names, generating them randomly to descrease any chance of a "lucky name". This time I've gone the other way! I'll run up a quick patch to fix this.
6 participants
bpo-40915: Fix mmap resize bugs on Windows
The current implementation of
mmap.resizeon Windows contains several subtle bugs. This PR, principally using code contributed by eryksun, addresses those bugs and adds corresponding tests.I've added some doc changes, but I'm not convinced that they should go in, since the cases addressed are relatively niche.
https://bugs.python.org/issue40915