PG1204 commented on PR #1249: URL: https://github.com/apache/arrow-java/pull/1249#issuecomment-5145252235
> @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 relying on a native leak-checker (ASAN/LSAN), keeping the current test as a graceful-failure guard? Happy either way. Just want to avoid landing a flaky test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
