Skip to content

Keep the original allocation valid when realloc wrapping fails - #5

Merged
riccardobl merged 1 commit into
NostrGameEngine:masterfrom
toaster0123:fix/transactional-realloc-20261005
Oct 5, 2026
Merged

riccardobl merged 1 commit into
NostrGameEngine:masterfrom
toaster0123:fix/transactional-realloc-20261005

Conversation

@toaster0123

Copy link
Copy Markdown
Contributor

Problem

High-level SaferAlloc.realloc resizes native memory before creating the replacement Java ByteBuffer. If wrapper creation throws OutOfMemoryError, the original allocation may already have moved or been freed. This violates the documented guarantee that the original remains valid when realloc throws.

Change

Add a package-private JNI entry point for the high-level API. It allocates a replacement, creates its Java wrapper, copies min(old capacity, new size) bytes, and only then frees the original. Wrapper failure releases the replacement and preserves the original. Negative sizes and heap buffers are rejected before mutation; null input and zero size are covered explicitly. Raw native realloc and exported function pointers retain their existing behavior.

Deliberate tradeoff

The high-level API now copies and temporarily holds both native allocations, including for shrink or same-size calls. It gives up in-place resizing to maintain the exception-safety guarantee. README documents this cost, ownership requirements, null/zero behavior, and position/limit semantics.

Regression evidence

With the original master 6439892 and a warmed realloc path, the isolated 16 MiB heap regression fails safely: native accounting changed after wrapper failure: 16 -> 4096. It checks accounting before dereferencing/freeing the original. With the fix, the original address, position, limit, contents, writeability and eventual free all remain valid, and accounting returns to baseline.

Validation

  • 15 selected JUnit tests passed: six ordinary realloc regressions, isolated heap-OOM regression, three existing smoke tests, and five extraction tests
  • Final heap-OOM regression passed 20 consecutive runs under -Xcheck:jni
  • Strict C compilation and Java source/target 8 compilation passed
  • Built actual Linux x86_64 JNI with GCC and pinned secure mimalloc v2.2.7
  • Raw function-pointer allocation/reallocation/free smoke test passed; existing native code is unchanged except for the new JNI entry point
  • git diff --check passed

Limits

Full Gradle/CMake and non-Linux platforms were not run locally. Java --release 8 platform data is unavailable in the cloud runtime, so source/target compilation is not equivalent to a complete JDK 8 compatibility check. No mobile-device runtime validation. This independent PR does not include the new-allocation wrapper leak fix or Darwin alias fix.

@riccardobl
riccardobl marked this pull request as ready for review October 5, 2026 20:07
@riccardobl
riccardobl merged commit 44aec6c into NostrGameEngine:master Oct 5, 2026
5 of 9 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.

2 participants