Skip to content

Fix #215: correct NIO buffer for Netty ByteBuf reads (view + copy, pick one) - #290

Open
bernardladenthin wants to merge 3 commits into
lmdbjava:masterfrom
bernardladenthin:fix/215-netty-niobuffer
Open

Fix #215: correct NIO buffer for Netty ByteBuf reads (view + copy, pick one)#290
bernardladenthin wants to merge 3 commits into
lmdbjava:masterfrom
bernardladenthin:fix/215-netty-niobuffer

Conversation

@bernardladenthin

@bernardladenthin bernardladenthin commented Jul 24, 2026

Copy link
Copy Markdown

Fixes #215, where ByteBuf.nioBuffer() on a value returned via ByteBufProxy.PROXY_NETTY yields zeros.

Root cause (confirming @2018ik)

For zero copy, a read-returned ByteBuf is repointed at LMDB's mmap by overwriting its memoryAddress (ByteBufProxy.out). The ByteBuf's own accessors (getBytes, …) then read the LMDB data correctly, but ByteBuf.nioBuffer() derives its NIO buffer from Netty's separate, chunk-shared backing buffer, which was never repointed — so it reads zeros. That chunk buffer cannot simply be repointed (as @2018ik suggested): it is shared by every pooled buffer carved from the same chunk, so overwriting its address would corrupt sibling buffers.

This is a "pick one" PR

Rather than guess, I've implemented both reasonable fixes as separate commits so you can choose and drop the other:

ByteBufProxy.nioBufferView ByteBufProxy.nioBufferCopy
mechanism zero-copy alias of LMDB mmap (same Unsafe address/capacity trick ByteBufferProxy already uses) copy out via ByteBuf.getBytes into a fresh direct buffer
cost none one copy
lifetime valid only while the txn is open (misuse → SIGSEGV, documented) outlives the txn/Env close
requires --add-opens java.base/java.nio yes (lazily, only if called) no
risk a foot-gun if misused robust

Commits:

  1. test(#215) — repro test pinning the current (buggy) behaviour.
  2. feat(#215)nioBufferView (zero-copy) + test.
  3. feat(#215)nioBufferCopy (safe copy) + tests.

Keep both, or drop commit 3 (view only) or commit 2 (copy only). I'll remove the unwanted variant (and its one cross-reference) in a follow-up.

Recommendation

My personal preference is to keep both nioBufferView and nioBufferCopy and document the trade-off clearly (the table above), so callers can pick per use case: zero-copy when the buffer is consumed within the transaction, or the safe copy when the bytes must outlive it. To me that is the cleanest outcome — the two genuinely serve different needs, and offering both (with the lifetime caveat spelled out) is more useful than forcing one.

That said, this is entirely your call. If you'd rather minimise API surface, I'm happy to keep just one — I'd suggest nioBufferCopy as the safe default — or to land only the repro test + docs and add no new API at all.

Notes / testing

  • New ByteBufNioBufferTest (4 tests, all green): view reflects data, copy reflects data, copy survives txn+env close (the key discriminator), and the raw nioBuffer() bug is documented.
  • Existing ByteBufProxyTest unaffected; fmt-maven-plugin:check clean.
  • I did not touch the Netty 4.2 / adaptive-allocator question (Netty 4.2 series and the adaptive allocator. #261); this fix applies to the current pooled path and the same lifecycle reasoning would carry over to an adaptive proxy.

bernardladenthin and others added 3 commits July 24, 2026 10:04
…le data

A value ByteBuf returned via ByteBufProxy.PROXY_NETTY reads correctly through the
ByteBuf's own accessors, but ByteBuf.nioBuffer() does not reflect the stored data
(it views Netty's separate, chunk-shared backing buffer, which the zero-copy read
path never repoints). This test pins the current behaviour; the following commits
add helpers to obtain a correct NIO buffer.

Refs lmdbjava#215.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01GdiZ3ABYsVHXBRNCkEizpE
Adds a static helper that materialises a read-only NIO ByteBuffer aliasing the
LMDB memory referenced by a PROXY_NETTY value ByteBuf, using the same Unsafe
address/capacity technique ByteBufferProxy already uses for its own zero-copy NIO
buffers. This lets Netty users hand a correct NIO buffer to NIO-based consumers
(compression, hashing, ...) instead of getting the zeros that ByteBuf.nioBuffer()
returns.

The java.nio.Buffer field offsets are resolved lazily in a holder so a JVM lacking
--add-opens java.base/java.nio only fails if nioBufferView is actually called,
rather than breaking PROXY_NETTY initialisation for everyone.

The returned buffer aliases LMDB-owned, read-only memory and is valid only while
the owning read Txn is open — documented prominently, as misuse is a JVM crash.

Refs lmdbjava#215.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01GdiZ3ABYsVHXBRNCkEizpE
…ative

Adds a second static helper as an alternative to nioBufferView: instead of
aliasing LMDB memory, it copies the readable bytes out via the ByteBuf's own
(correct) accessor into a fresh direct NIO buffer. This uses no Unsafe/reflection,
needs no --add-opens, and — unlike the view — remains valid after the owning
transaction (or the Env) is closed.

The two helpers are deliberately offered together so the maintainers can choose:
keep the zero-copy view, the safe copy, or both. Dropping this commit removes the
copy variant; dropping the previous commit removes the view. Recommendation:
nioBufferCopy as the safe default, nioBufferView for zero-copy hot paths.

Refs lmdbjava#215.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01GdiZ3ABYsVHXBRNCkEizpE
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.

ByteBuf.nioBuffer() returns a buffer with all zeros

1 participant