Skip to content

gh-73126: Don't fail early when __length_hint__() is too big for preallocation. - #8635

Closed
sir-sigurd wants to merge 2 commits into
python:mainfrom
sir-sigurd:length-hint
Closed

gh-73126: Don't fail early when __length_hint__() is too big for preallocation.#8635
sir-sigurd wants to merge 2 commits into
python:mainfrom
sir-sigurd:length-hint

Conversation

@sir-sigurd

@sir-sigurd sir-sigurd commented Aug 2, 2018

Copy link
Copy Markdown
Contributor

Comment thread Objects/abstract.c Outdated
if (result == NULL) {
/* bpo-28940 - LengthHint could lie and asked for too much,
* try again without preallocation. */
PyErr_Clear();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will silence any possible exception raised in PyTuple_New() (same for PyByteArray_FromStringAndSize(), _PyBytesWriter_Alloc(), etc.). I don't think that's the right approach.

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Dec 15, 2024
@picnixz picnixz changed the title bpo-28940: Don't fail early when __length_hint__() is too big for preallocation. gh-73126: Don't fail early when __length_hint__() is too big for preallocation. Dec 15, 2024
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Feb 27, 2025
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 13, 2025
# Conflicts:
#	Lib/test/test_bytes.py
#	Lib/test/test_tuple.py
#	Objects/abstract.c
#	Objects/bytesobject.c
#	Objects/listobject.c
@serhiy-storchaka

Copy link
Copy Markdown
Member

The problem with this approach is that it only helps when the allocation fails immediately. A hint which is not large enough to fail, but large enough to exhaust memory and swap, is worse, and it would not be handled.

There are three alternatives:

  • Do nothing: the correctness of __length_hint__() is the responsibility of the class which implements it, and we do not protect against other protocol methods lying either.
  • Limit the preallocation to some reasonable value.
  • Do not use the hint for preallocation at all, as PySequence_Tuple() already does.

Thank you for the patch anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants