Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove pinned buffer list in JNI wrapper to avoid segmentation faults #697

Merged
merged 2 commits into from
May 10, 2023
Merged

Conversation

ShukantPal
Copy link
Contributor

Fixes #690

After going through the C++ code in libktx, I realized that ktxTexture{1,2}_setImageFromMemory copy from the source buffers anyway. That meant pinning the source byte arrays in Java didn't do anything useful; hence, I removed that and the associated code keeping track of pinned buffers.

I have also added a unit test to make sure the JNI wrapper is stable for ~1000 iterations of usage.

@MarkCallow
Copy link
Collaborator

Thank you @ShukantPal. You are correct that ktxTexture{1,2}_SetImageFromMemory copy from source.

@MarkCallow MarkCallow merged commit 9b084d5 into KhronosGroup:main May 10, 2023
@robnugent
Copy link

@ShukantPal - first off, I just wanted to say many thanks again for looking into this problem.

FYI - as a heads-up for you, I think there is still a related issue. I built the latest code (with your fix) on Windows/ARM and Windows/x64 and when I take my multithreaded test from issue #690 and increase the number of iterations, then it looks like it's leaking memory. (This can be seen from Windows Task Manager).

The test eventually crashes the JVM on both platforms, where it should run OK forever, Note that this is not normal Java Heap memory variation, as if I limit that with e.g. the JVM flag -Xmx512m, then it still fails with a JVM crash. even consuming all the main memory and swap on my Windows/x64 box, which has 128GB RAM and 128GB swap.

I'm not confident in my own build of toktx (not sure I specified all the right build options), so I'm wont raise a bug at this stage, but I will do if the problem persists when I get an official 4.1.1 (or whatever) toktx build with your fix.

Thanks again
Rob

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.

Crash when using Java interface on Windows
3 participants