Skip to content

Harden Unpacker.__init__ re-entry cleanup to prevent buffer/context leaks#687

Merged
methane merged 3 commits into
mainfrom
copilot/fix-unpacker-init-bugs
Jun 3, 2026
Merged

Harden Unpacker.__init__ re-entry cleanup to prevent buffer/context leaks#687
methane merged 3 commits into
mainfrom
copilot/fix-unpacker-init-bugs

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 3, 2026

Unpacker.__init__ did not safely handle repeated initialization: it replaced self.buf without freeing the previous allocation and reinitialized parser state without clearing in-flight stack objects. This PR makes re-entry cleanup explicit and deterministic.

  • Unpacker lifecycle cleanup

    • Initialize parser context in __cinit__ (unpack_init(&self.ctx)) so cleanup paths always operate on initialized state.
    • On every __init__ entry:
      • clear parse-stack state (unpack_clear(&self.ctx)),
      • reset parser bookkeeping (unpack_init(&self.ctx)),
      • free old internal buffer when present (PyMem_Free(self.buf)),
      • null out the pointer before reallocating (self.buf = NULL).
  • Regression coverage for re-initialization

    • Add a focused unpacker test that drives a mid-parse state (live object held in parse stack), re-calls __init__, and asserts:
      • prior stack-held object is released,
      • parser is reset to empty state,
      • unpacker remains reusable for new input.
# __init__ re-entry prologue
unpack_clear(&self.ctx)
unpack_init(&self.ctx)
if self.buf != NULL:
    PyMem_Free(self.buf)
    self.buf = NULL
Original prompt

Create a pull request in repository msgpack/msgpack-python to fix the two Unpacker.__init__ re-initialization bugs described in issue #683.

Repository:

  • msgpack/msgpack-python

Issue context:

  • Issue A (tentative) TODO list? (Generated from Claude Code plugins) #683 reports two related bugs in Unpacker.__init__:
    1. Unpacker.__init__ overwrites self.buf via PyMem_Malloc without first freeing the existing buffer, so re-calling __init__ (including subclass super().__init__(...)) leaks the previous allocation.
    2. Unpacker.__init__ calls unpack_init rather than clearing existing parser state first, so any live PyObject* references in ctx->stack[1..top] are leaked when re-initializing a mid-stream Unpacker.

Requested change:

  • Update the Unpacker.__init__ implementation in the Cython source so that, before allocating a new buffer or reinitializing the unpack context, it safely cleans up any existing state.
  • Specifically, if the instance already has an allocated buffer / active unpack context, clear parser state with unpack_clear(&self.ctx), free the old buffer with PyMem_Free(self.buf), and reset self.buf appropriately before continuing initialization.

Requirements:

  1. Make the fix in the relevant Cython source file for Unpacker.
  2. Ensure the cleanup path is safe for first initialization as well as repeated __init__ calls.
  3. Keep the PR focused on these two Unpacker.__init__ bugs only.
  4. Regenerate checked-in generated files only if the repository expects that for Cython source changes.
  5. Add or update tests if there is an appropriate place to cover re-initialization behavior.
  6. Open a pull request that references issue A (tentative) TODO list? (Generated from Claude Code plugins) #683 and explains that it fixes both the buffer leak and parse-stack leak on Unpacker.__init__ re-entry.

Suggested validation:

  • Run relevant tests for unpacker behavior.
  • If practical, add a regression test covering repeated __init__ calls or subclass reinitialization.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Packer.__getbuffer____releasebuffer__@cython.critical_section を追加するPRを作って。

Assistant: PR作成の確認待ちです。GitHub 側の承認が完了したら、その変更で続けて進めます。

User: @copilot Accepted Confirmation: Confirm cloud agent session

Assistant: 開始しました。進捗はタスクページで追えます。

User: "Unpacker.init overwrites self.buf via PyMem_Malloc without first freeing" と "Unpacker.init calls unpack_init (not unpack_clear) before re-initializing" を修正するPRを作成して。

Assistant: 確認待ちです。GitHub の承認ダイアログで許可してもらえれば、この PR 作成を続けます。

This pull request was created from Copilot chat.

Copilot AI changed the title [WIP] Fix Unpacker.__init__ re-initialization bugs Harden Unpacker.__init__ re-entry cleanup to prevent buffer/context leaks Jun 3, 2026
Copilot AI requested a review from methane June 3, 2026 07:21
Comment thread msgpack/_unpacker.pyx Outdated
@methane methane marked this pull request as ready for review June 3, 2026 07:36
@methane methane merged commit 77395c1 into main Jun 3, 2026
36 checks passed
@methane methane deleted the copilot/fix-unpacker-init-bugs branch June 3, 2026 07:40
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.

2 participants