Fix #215: correct NIO buffer for Netty ByteBuf reads (nioBufferView + nioBufferCopy) by bernardladenthin · Pull Request #290 · lmdbjava/lmdbjava

bernardladenthin · 2026-07-24T08:09:08Z

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.

Fix: two complementary helpers

Following review, both are kept:

	`nioBufferView(ByteBuf)`	`nioBufferCopy(ByteBuf)` / `nioBufferCopy(ByteBuf, IntFunction<ByteBuffer>)`
mechanism	zero-copy alias of LMDB mmap (same `Unsafe` address/capacity trick as `ByteBufferProxy`), via a reused per-thread base buffer	copy via `ByteBuf.getBytes` into a new direct buffer, or into a caller-supplied (e.g. pooled) buffer
cost	no copy; one read-only wrapper object per call	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

Commits:

test(#215) — repro test pinning the current (buggy) behaviour.
feat(#215) — nioBufferView (zero-copy) + test.
feat(#215) — nioBufferCopy (safe copy) + tests.
feat(#215) — review: nioBufferCopy supplier overload; nioBufferView reuses a per-thread base buffer.

Notes / testing

ByteBufNioBufferTest (9 tests): view/copy reflect data, copy survives txn+env close, earlier views unaffected by later calls, supplier contract (exact size, position/flip, untouched tail, too small → IAE, null → NPE), raw nioBuffer() bug documented.
Full suite 1124/1124, mvn verify and fmt:check clean; new code fully covered.
Merges cleanly with fix(#261): support heap (address-less) ByteBuf in ByteBufProxy.in() #291 (same file); the combined tree passes.
Netty 4.2 / adaptive allocator (Netty 4.2 series and the adaptive allocator. #261) untouched.

…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

at055612 · 2026-08-14T12:03:13Z

Thanks for the PR.

I don't see any reason not to add both of these utility methods.

However, I think nioBufferCopy() should have two overloaded forms like this:

  public static ByteBuffer nioBufferCopy(final ByteBuf buffer)
  
  public static ByteBuffer nioBufferCopy(
      final ByteBuf buffer,
      final IntFunction<ByteBuffer> destinationBufferSupplier)

This allows the caller to use a buffer they already have or can obtained from a pool under their control.

Similarly, nioBufferView probably ought to use the ThreadLocal buffers queue approach to be consistent with org.lmdbjava.ByteBufferProxy.AbstractByteBufferProxy#allocate, so save repeated allocation of a zero length buffer, which I think has non-trivial cost.

… reuse view base Adds nioBufferCopy(ByteBuf, IntFunction<ByteBuffer>) so callers can copy into a buffer they already have or obtain from their own pool. The supplier is given the readable size and must return a buffer with at least that many bytes remaining; the bytes are written at its position and the buffer is returned flipped. The one-arg form now delegates with ByteBuffer::allocateDirect. nioBufferView no longer allocates a zero-length direct buffer per call: it repoints a per-thread base buffer (ThreadLocal, as in ByteBufferProxy) and returns asReadOnlyBuffer(), which snapshots the address/capacity, so earlier views are unaffected by later calls on the same thread. Refs lmdbjava#215. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01L4Qkne1apX58veQ6bY3w84

bernardladenthin · 2026-10-14T15:59:08Z

Thanks @at055612 — done in 80b8abf:

Added nioBufferCopy(ByteBuf, IntFunction<ByteBuffer>); the one-arg form now delegates with ByteBuffer::allocateDirect. The supplier is given readableBytes() and must return a buffer (heap or direct) with at least that many bytes remaining; the bytes are written at its position and the buffer is returned flipped. Too small → IllegalArgumentException, null → NPE.
nioBufferView now reuses a per-thread base buffer (ThreadLocal, as in AbstractByteBufferProxy) instead of allocateDirect(0) per call. I used a single base per thread rather than a queue, since the view is handed to the caller and never comes back via a deallocate. This is safe because asReadOnlyBuffer() copies the address/capacity into the new view, so repointing the base on a later call doesn't affect earlier views — covered by a new test. The only per-call allocation left is the read-only wrapper object.

5 new tests in ByteBufNioBufferTest; full suite 1124/1124 green, fmt:check clean.

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

bernardladenthin mentioned this pull request Jul 24, 2026

Netty 4.2 series and the adaptive allocator. #261

Open

bernardladenthin changed the title ~~Fix #215: correct NIO buffer for Netty ByteBuf reads (view + copy, pick one)~~ Fix #215: correct NIO buffer for Netty ByteBuf reads (nioBufferView + nioBufferCopy) Oct 14, 2026

Navigation Menu

Repository navigation

Fix #215: correct NIO buffer for Netty ByteBuf reads (nioBufferView + nioBufferCopy) - #290

Fix #215: correct NIO buffer for Netty ByteBuf reads (nioBufferView + nioBufferCopy)#290
bernardladenthin wants to merge 4 commits into
lmdbjava:masterfrom
bernardladenthin:fix/215-netty-niobuffer

bernardladenthin commented Jul 24, 2026 •

edited

Loading

Uh oh!

at055612 commented Aug 14, 2026

Uh oh!

bernardladenthin commented Oct 14, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bernardladenthin commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Root cause (confirming @2018ik)

Fix: two complementary helpers

Notes / testing

Uh oh!

at055612 commented Aug 14, 2026

Uh oh!

bernardladenthin commented Oct 14, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bernardladenthin commented Jul 24, 2026 •

edited

Loading