Skip to content

gh-151815: Fix crash in t-string iterator on partial construction under OOM#154714

Closed
Vamsi-klu wants to merge 1 commit into
python:mainfrom
Vamsi-klu:fix-gh151815-template-iter-oom
Closed

gh-151815: Fix crash in t-string iterator on partial construction under OOM#154714
Vamsi-klu wants to merge 1 commit into
python:mainfrom
Vamsi-klu:fix-gh151815-template-iter-oom

Conversation

@Vamsi-klu

Copy link
Copy Markdown

Summary

Fix OOM-0024 (gh-151815 / umbrella gh-151763): crash when constructing a t-string / template iterator fails partway under memory pressure because templateiterobject fields were left uninitialized before a fallible PyObject_GetIter.


What the issue is

template_iter does:

templateiterobject *iter = PyObject_GC_New(...);
// fields UNINITIALIZED
PyObject *stringsiter = PyObject_GetIter(self->strings);
if (stringsiter == NULL) {
    Py_DECREF(iter);   // -> tp_dealloc -> tp_clear -> Py_CLEAR(self->stringsiter)
    return NULL;
}

PyObject_GC_New does not zero the struct. On the error path, templateiter_clear runs Py_CLEAR on garbage stringsiter / interpolationsiter pointers → crash / heap corruption under fusil-style allocation failure (OOM-0024).


Why I solved it that way

  1. NULL-init immediately after allocation, before any fallible call — the standard CPython partially-constructed-object pattern (same family as several already-merged OOM fixes under CPython crashes on memory-allocation failure (OOM): 35 findings (4 dups → 31 distinct) #151763).
  2. Prefer NULL-init over switching error paths to PyObject_GC_Del, because templateiter_clear already uses Py_CLEAR (NULL-safe). Keeping dealloc unified avoids a second free path.
  3. Also set from_strings = 1 early so the object is fully consistent if anything inspects it during cleanup.
  4. Minimal diff — no API change, no generator change, no test harness required for merge (sibling OOM fixes often shipped without tests; behavior is "don't crash on OOM").

How I did it

Objects/templateobject.c in template_iter, after successful PyObject_GC_New:

iter->stringsiter = NULL;
iter->interpolationsiter = NULL;
iter->from_strings = 1;

Then the existing GetIter assignments overwrite those fields on success.

NEWS: Misc/NEWS.d/next/Core_and_Builtins/…gh-issue-151815….rst


Impact

  • Removes a real crash under allocation failure when iterating string.templatelib.Template / t-strings.
  • No change on the success path beyond three stores that were about to happen anyway.
  • Candidate for backport wherever t-strings / template objects exist (3.14+).

Testing plan

  • Full rebuild of the debug interpreter.
  • ./python -m test test_string.test_templatelib — PASS.
  • Manual smoke: iterate a Template with interpolations — OK.
  • Optional: _testcapi.set_nomemory sweep around template_iter (not added; happy to add if reviewers want a locked-in repro).
  • CI: standard build + test matrix.

Everything else


Requested reviewers (subject-matter experts)

Could the following SMEs take a look when convenient (I cannot formally request reviews from this fork account):

Thank you!

…on under OOM

Initialize templateiter fields to NULL immediately after PyObject_GC_New so
tp_clear/dealloc on the error path does not clear garbage pointers
(OOM-0024 / pythongh-151763).
@Vamsi-klu
Vamsi-klu requested a review from lysnikolaou as a code owner July 26, 2026 07:13
@python-cla-bot

python-cla-bot Bot commented Jul 26, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 277cbd6aca

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Vamsi-klu
Vamsi-klu marked this pull request as draft July 26, 2026 08:01
@Vamsi-klu

Copy link
Copy Markdown
Author

Closing this PR. Thank you for the review attention.

@Vamsi-klu Vamsi-klu closed this Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant