GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-138205: Remove the resize method on mmap object on platforms don't support it by aisk · Pull Request #138276 · python/cpython

Conversation

aisk commented Aug 31, 2025 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

Issue: mmap.resize() can raise SystemError #138205

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Remove the resize method on mmap object on platforms don't support it

e3c9987

bedevere-app Bot added the awaiting review label

Aug 31, 2025

bedevere-app Bot mentioned this pull request

Aug 31, 2025

mmap.resize() can raise SystemError #138205

Closed

aisk added 5 commits

August 31, 2025 12:43


          Fix rst markers

2faa469


          Fix rst markers

a251dea


          Update related document

95ce088


          Revert the document changes which not related to this PR

42e562e


          Fix docs

e08de1c

serhiy-storchaka approved these changes

Sep 3, 2025

View reviewed changes

serhiy-storchaka left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your PR, @aisk. Actually, I already had a patch, I just waited until other mmap issues were resolved. It is very similar to your PR, so let's continue with it.

Please update your PR, few new tests that use SystemError were added.

I think it is worth to add an entry in the "Porting to 3.15" section in What's New.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_mmap.py Outdated

		@unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize')
		def test_resize(self):
		# Create a file to be mmap'ed.
		f = open(TESTFN, 'bw+')

serhiy-storchaka Sep 3, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can simply use with open(...) as f.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_mmap.py Outdated

Comment on lines +139 to +140

		f = open(TESTFN, 'rb')
		try:

serhiy-storchaka Sep 3, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_mmap.py Outdated

		try:
		m.resize(2*mapsize)
		except SystemError: # resize is not universally supported
		except AttributeError: # resize is not universally supported

serhiy-storchaka Sep 3, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-            # Ensuring that readonly mmap can't be resized
-            try:
-                m.resize(2*mapsize)
-            except SystemError:   # resize is not universally supported
-                pass
-            except TypeError:
-                pass
-            else:
-                self.fail("Able to resize readonly memory map")
+            if hasattr(m, 'resize'):
+                # Ensuring that readonly mmap can't be resized
+                with self.assertRaises(TypeError):
+                    m.resize(2*mapsize)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_mmap.py Outdated

		try:
		m.resize(512)
		except SystemError:
		except AttributeError:

serhiy-storchaka Sep 3, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply use if hasattr(m, 'resize').

-            # Try resizing map
-            try:
+            if hasattr(m, 'resize'):
                 m.resize(512)
-            except SystemError:
-                pass
-            else:
-                # resize() is supported

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/mmapmodule.c Outdated

		return 0;

		}
		#endif /* defined(MS_WINDOWS) \|\| defined(HAVE_MREMAP) */

serhiy-storchaka Sep 3, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually write this simpler:

Suggested change

	#endif /* defined(MS_WINDOWS) \|\| defined(HAVE_MREMAP) */
	#endif /* MS_WINDOWS \|\| HAVE_MREMAP */

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/mmapmodule.c Outdated

		#endif /* UNIX */
		}
		}
		#endif /* defined(MS_WINDOWS) \|\| defined(HAVE_MREMAP) */

serhiy-storchaka Sep 3, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Doc/library/mmap.rst Outdated

		pagefile) will silently create a new map with the original data copied over
		up to the length of the new size.

		.. availability:: Linux, Windows

serhiy-storchaka Sep 3, 2025

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also available on other platforms (e.g. FreeBSD).

See how this is documented for madvise().

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot added awaiting merge and removed awaiting review labels

Sep 3, 2025

aisk added 3 commits

September 3, 2025 23:44


          Update by comments

ea35d4b


          Fix marco

2413ceb


          Merge remote-tracking branch 'upstream/main' into remove-mmap-resize

b1241ab

aisk force-pushed the remove-mmap-resize branch from 91cda99 to b1241ab Compare

September 3, 2025 15:05

aisk commented Sep 3, 2025

Copy link

Copy Markdown

Member Author

Sorry for the force-push. I messed up the conflict resolution during the merge and had to redo the merge after the push.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aisk and others added 2 commits

September 4, 2025 00:20


          Fix tests with platform without mmap.resize

1d36a44


          Update 3.15.rst

590523f

aisk requested a review from AA-Turner as a code owner

September 4, 2025 03:17

serhiy-storchaka approved these changes

Sep 4, 2025

View reviewed changes

serhiy-storchaka left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Doc/library/mmap.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

aisk and others added 2 commits

September 4, 2025 14:21


          Update Doc/library/mmap.rst

0ad3c26

Co-authored-by: Serhiy Storchaka <[email protected]>


          Merge branch 'main' into remove-mmap-resize

e4ab905

vstinner approved these changes

Sep 4, 2025

View reviewed changes

vstinner left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_mmap.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Lib/test/test_mmap.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

aisk and others added 2 commits

September 5, 2025 01:12


          Update Lib/test/test_mmap.py

b44d044

Co-authored-by: Victor Stinner <[email protected]>


          Update Lib/test/test_mmap.py

a73e936

Co-authored-by: Victor Stinner <[email protected]>

kumaraditya303 approved these changes

Sep 6, 2025

View reviewed changes

kumaraditya303 merged commit c919d02 into python:main

Sep 6, 2025

bedevere-app Bot removed the awaiting merge label

Sep 6, 2025

picnixz mentioned this pull request

Sep 7, 2025

gh-57911: Sanitize symlink targets in tarfile on win32 #138309

Merged

lkollar pushed a commit to lkollar/cpython that referenced this pull request

Sep 9, 2025


          pythongh-138205: Remove the resize method on mmap object on platf…

5da862a

…orms don't support it (python#138276)

Co-authored-by: Serhiy Storchaka <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>

salty-horse mentioned this pull request

Jan 5, 2026

gh-138205: explicit mention to mmap.mmap.resize in "Porting to Python 3.15" notes #143440

Merged

JelleZijlstra mentioned this pull request

May 9, 2026

Add Python 3.15 socket SSL and platform updates python/typeshed#15734

Merged

aisk deleted the remove-mmap-resize branch

June 20, 2026 10:41

Reviewers

vstinner

vstinner approved these changes

serhiy-storchaka

serhiy-storchaka approved these changes

kumaraditya303

kumaraditya303 approved these changes

AA-Turner

Awaiting requested review from AA-Turner

AA-Turner is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

aisk commented Aug 31, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aisk commented Sep 3, 2025

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aisk commented Aug 31, 2025 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.