Skip to content

gh-137928: remove redundant size validation in multiprocessing.heap #137929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gpshead merged 3 commits into from
Nov 12, 2025
Merged

gh-137928: remove redundant size validation in multiprocessing.heap #137929

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8c3baf0
gh-137928: Centralize size validation in multiprocessing.heap
acabelloj Aug 18, 2025
204d264
gh-137928: Move size validation helper to module level in multiproces…
acabelloj Aug 21, 2025
585f90f
remove redundant size check, malloc does it
gpshead Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions Lib/multiprocessing/heap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,15 @@ def _roundup(n, alignment):
mask = alignment - 1
return (n + mask) & ~mask

@staticmethod
# Bind sys.maxsize once at function definition time to avoid global lookups.
def _validate_size(size, _maxsize=sys.maxsize):
"""Validate requested size; raise ValueError if < 0, OverflowError if >= _maxsize."""
if size < 0:
raise ValueError("Size {0:n} out of range".format(size))
if _maxsize <= size:
raise OverflowError("Size {0:n} too large".format(size))

def _new_arena(self, size):
# Create a new arena with at least the given *size*
length = self._roundup(max(self._size, size), mmap.PAGESIZE)
Expand Down Expand Up @@ -295,10 +304,7 @@ def free(self, block):

def malloc(self, size):
# return a block of right size (possibly rounded up)
if size < 0:
raise ValueError("Size {0:n} out of range".format(size))
if sys.maxsize <= size:
raise OverflowError("Size {0:n} too large".format(size))
Heap._validate_size(size)
Copy link
Copy Markdown
Contributor

@maurycy maurycy Aug 20, 2025

Choose a reason for hiding this comment

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

Suggested change
Heap._validate_size(size)
self._validate_size(size)

Copy link
Copy Markdown
Contributor Author

@acabelloj acabelloj Aug 21, 2025

Choose a reason for hiding this comment

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

This will no longer apply when moving it to the module level, but thanks for the suggestion

if os.getpid() != self._lastpid:
self.__init__() # reinitialize after fork
with self._lock:
Expand All @@ -324,10 +330,7 @@ class BufferWrapper(object):
_heap = Heap()

def __init__(self, size):
if size < 0:
raise ValueError("Size {0:n} out of range".format(size))
if sys.maxsize <= size:
raise OverflowError("Size {0:n} too large".format(size))
Heap._validate_size(size)
Copy link
Copy Markdown
Contributor

@maurycy maurycy Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe it's better to move the _validate_size to the module level, to avoid coupling between BufferWrapper and a private method?

Copy link
Copy Markdown
Contributor Author

@acabelloj acabelloj Aug 21, 2025

Choose a reason for hiding this comment

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

Yes, that was one of the options I had in mind. Thinking it over, moving it to the module level does reduce the coupling. I'll update the PR accordingly.

block = BufferWrapper._heap.malloc(size)
self._state = (block, size)
util.Finalize(self, BufferWrapper._heap.free, args=(block,))
Expand Down
Loading