Skip to content

gh-149183: avoid MemoryError in tokenize#149445

Open
KowalskiThomas wants to merge 5 commits into
python:mainfrom
KowalskiThomas:kowalski/fix-avoid-memoryerror-in-tokenize
Open

gh-149183: avoid MemoryError in tokenize#149445
KowalskiThomas wants to merge 5 commits into
python:mainfrom
KowalskiThomas:kowalski/fix-avoid-memoryerror-in-tokenize

Conversation

@KowalskiThomas

@KowalskiThomas KowalskiThomas commented May 6, 2026

Copy link
Copy Markdown
Contributor

@read-the-docs-community

read-the-docs-community Bot commented May 6, 2026

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #32634426 | 📁 Comparing 9203a8e against main (970b043)

  🔍 Preview build  

65 files changed · ± 64 modified · - 1 deleted

± Modified

- Deleted

@StanFromIreland StanFromIreland changed the title fix: avoid MemoryError in tokenize gh-149183: avoid MemoryError in tokenize May 6, 2026
Comment thread Parser/lexer/lexer.c Outdated
Comment thread Lib/test/test_tokenize.py Outdated
@KowalskiThomas KowalskiThomas marked this pull request as ready for review May 11, 2026 14:48
@KowalskiThomas KowalskiThomas requested a review from picnixz May 11, 2026 14:48
Comment thread Parser/lexer/lexer.c Outdated
size tracking to underflow. Treat as a tokenizer error rather
than passing a negative length (cast to huge size_t) to malloc or
PyUnicode_DecodeUTF8. */
return -1;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we set tok->done or raise a tokenizer syntax error before returning here? Otherwise this reports unknown tokenization error and compile() gets unknown parsing error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll look into that. Thanks.

Comment thread Parser/lexer/lexer.c Outdated
size tracking to underflow. Treat as a tokenizer error rather
than passing a negative length (cast to huge size_t) to malloc or
PyUnicode_DecodeUTF8. */
return -1;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We return -1 here without setting tok->done or an exception, so the user ends up with the generic unknown tokenization error (and unknown parsing error from the compiler) instead of something descriptive. Should we set tok->done = E_TOKEN or call _PyTokenizer_syntaxerror so the message points at the actual problem?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(I believe this is the same suggestion as your other comment, I'll try to do it the way you suggest 🙇)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay so I did exactly what you suggested -- tok->done = E_TOKEN :)

@bedevere-app

bedevere-app Bot commented Jul 11, 2026

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@KowalskiThomas

Copy link
Copy Markdown
Contributor Author

Sorry in advance, I had to rebase on main (instead of merge) because Git wouldn't let me "merge unrelated histories"... Not sure what happened there...

@KowalskiThomas KowalskiThomas force-pushed the kowalski/fix-avoid-memoryerror-in-tokenize branch from 9203a8e to 1937551 Compare July 14, 2026 09:21
@KowalskiThomas KowalskiThomas requested a review from pablogsal July 15, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

t-string tokeniser reports MemoryError on invalid input

3 participants