Seems reasonable. From what I was researching, doesn't seem like it is a leak easy to hit and it may only leak a few KBs.

I'd probably just try to add a test that reproduces the issue an ensure that it passes with the fix

Seems reasonable. From what I was researching, doesn't seem like it is a leak easy to hit and it may only leak a few KBs.

I'd probably just try to add a test that reproduces the issue an ensure that it passes with the fix

@xborder Agreed it's low-impact and hard to hit. I've added a regression test (TestFromSchemaByteArray) that feeds malformed schema bytes to the native createDataset path, which is the exact branch that used to skip ReleaseByteArrayElements, and confirms it now fails gracefully with the fix in place.

@PG1204 I don't think this is testing that we fixed the memory leak. This is just testing the error is thrown. Shouldn't it check no memory was leaked?

@PG1204 I don't think this is testing that we fixed the memory leak. This is just testing the error is thrown. Shouldn't it check no memory was leaked?

@xborder you're right, that only proves the error path is hit, not that the leak is gone. The catch is what leaks: the buffer wraps the pointer from GetByteArrayElements, which on HotSpot copies the array into a JVM-internal malloc'd buffer, and that copy is what leaked. It's not tracked by NativeMemoryPool, not on the Java heap, and has no portable Java API; it's only visible via process RSS.

So the only way I see to assert "no leak" from Java is an RSS-growth check: loop the call many times with a large payload and assert RSS stays bounded (unfixed balloons by GBs, fixed stays flat). That works on Linux but needs gating on macOS/Windows and is a bit flaky.

Would you prefer (1) that RSS-based test, or (2) dropping the Java assertion and verifying manually with a local leak-checker (valgrind/ASAN), keeping the current test as a graceful-failure guard? Note CI doesn't run a sanitizer today, so option 2 wouldn't be enforced automatically. Happy either way, just want to avoid landing a flaky test.

I don't think that's reasonable to do. I doubt this is a code path that gets exercised often anyways?

I don't think that's reasonable to do. I doubt this is a code path that gets exercised often anyways?

agreed, thanks. I'll skip the RSS-based test then, it'd be flaky and isn't worth it for this path. I'll keep the RAII fix plus the existing lightweight test as a graceful-failure regression guard (malformed schema bytes -> exception, no crash), and I'll reword its javadoc so it no longer claims to assert the leak itself. Let me know if you'd rather want to drop the test entirely.

@PG1204 can you rebase?

(Also, are you using AI assistance at all? I believe ASF may start requiring disclosure soon.)

@PG1204 can you rebase?

(Also, are you using AI assistance at all? I believe ASF may start requiring disclosure soon.)

@lidavidm rebase done.
yeah i'm using AI assistance, would you like me to disclose it in the PR description?