◐ Shell
clean mode source ↗

Harden `Unpacker.__init__` re-entry cleanup to prevent buffer/context leaks by Copilot · Pull Request #687 · msgpack/msgpack-python

Copilot

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.