Skip to content

Return the pooled buffer when encoding fails - #505

Merged
em3s merged 1 commit into
kakao:mainfrom
shin4141:codex/fix-pooled-buffer-release
Oct 2, 2026
Merged

em3s merged 1 commit into
kakao:mainfrom
shin4141:codex/fix-pooled-buffer-release

Conversation

@shin4141

@shin4141 shin4141 commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Summary

AbstractEdgeEncoder.useAsByteArray returned a borrowed EdgeBuffer only after the encoding callback and byte copy succeeded. If the callback threw, that buffer was permanently removed from the shared pool. With a one-buffer pool, a later valid encoding then had no buffer to borrow.

Return the buffer in finally. The original exception still reaches the caller, and the next use resets the returned buffer before writing. This change does not alter successful encodings.

Regression

The new test uses a one-buffer pool, writes one value, then asks the encoder to serialize an unsupported object. On main at 0cd8e45a4bd8f383bf58175e05fe51ba64f0c06b, it fails because the pool size is 0 instead of 1. With the patch, it checks that the original exception is preserved, the same buffer returns to the pool, and a subsequent valid encoding matches a fresh unpooled encoder.

Tests

  • ./gradlew :codec-java:test — 67 passed.
  • :core:test — 8,918 passed, 5 skipped; :core-java:test — 313 passed.
  • ./gradlew :server:test — 745 passed with a test-runtime Byte Buddy agent because the isolated macOS build disallows self-attachment.
  • ./gradlew spotlessCheck — passed.
  • ./gradlew test — not fully verified: the unrelated engine test fixture's BlockHound.install() cannot self-attach in the isolated macOS environment (Operation not permitted). The five DefaultBufferCapacityTest cases themselves passed.

AI Assistance

  • This change was written largely with AI assistance.
    • Tool / model: Codex (GPT-6).

@shin4141
shin4141 requested a review from em3s as a code owner October 1, 2026 00:41
@CLAassistant

CLAassistant commented Oct 1, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@em3s em3s left a comment •

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.

Thanks @shin4141 — the one-buffer test pins it exactly. Fails on main, passes with the patch.

I'll tweak the title and merge. One thing left over: borrow() is pool.poll(), so past poolSize concurrent encodes it returns null and reset() NPEs. Falling back to new EdgeBuffer() would cover it. Happy to take a follow-up PR if you're interested.

@em3s em3s changed the title fix(codec-java): return pooled buffer after encoding fails Return the pooled buffer when encoding fails Oct 2, 2026
@em3s
em3s merged commit afa7143 into kakao:main Oct 2, 2026
3 checks passed
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.

3 participants