Skip to content

Commit

Permalink
Remove pinned buffer list in JNI wrapper to avoid segmentation faults (
Browse files Browse the repository at this point in the history
…#697)

* Remove pinned buffer list in JNI wrapper to avoid segmentation faults

* Update interface/java_binding/src/test/java/org/khronos/ktx/test/KtxParallelTest.java
  • Loading branch information
ShukantPal authored May 10, 2023
1 parent d7255fe commit 9b084d5
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 83 deletions.
81 changes: 1 addition & 80 deletions interface/java_binding/src/main/cpp/KtxTexture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,83 +13,6 @@
#include "ktx.h"
#include "libktx-jni.h"

struct pinned_image_buf
{
jbyteArray handle;
jbyte *data;
};

// The buffer list is a vector of memory buffers pinned by KTXTexture. These buffers are pinned
// when setImageFromMemory is used. They are freed when KTXTexture.destroy is invoked.

static std::vector<pinned_image_buf> *get_or_create_buffer_list(JNIEnv *env, jobject thiz)
{
jclass ktx_texture_class = env->GetObjectClass(thiz);
jfieldID ktx_buffers_field = env->GetFieldID(ktx_texture_class, "buffers", "J");

std::vector<pinned_image_buf> *buffers =
reinterpret_cast<std::vector<pinned_image_buf>*>(env->GetLongField(thiz, ktx_buffers_field));

// Lazy init
if (buffers == NULL) {
buffers = new std::vector<pinned_image_buf>();
env->SetLongField(thiz, ktx_buffers_field, reinterpret_cast<jlong>(buffers));
}

return buffers;
}

static void push_buffer_list(JNIEnv *env, jobject thiz, jbyteArray handle, jbyte *data)
{
std::vector<pinned_image_buf> *buffers = get_or_create_buffer_list(env, thiz);

pinned_image_buf buf;

buf.handle = handle;
buf.data = data;

buffers->push_back(buf);
}

static void free_buffer_list(JNIEnv *env, jobject thiz)
{
jclass ktx_texture_class = env->GetObjectClass(thiz);
jfieldID ktx_buffers_field = env->GetFieldID(ktx_texture_class, "buffers", "J");

std::vector<pinned_image_buf> *buffers =
reinterpret_cast<std::vector<pinned_image_buf>*>(env->GetLongField(thiz, ktx_buffers_field));

// Nothing to free
if (buffers == NULL) {
return;
}

std::vector<pinned_image_buf> l_buffers = *buffers;

for (std::vector<pinned_image_buf>::iterator it = std::begin(l_buffers);
it != std::end(l_buffers);
++it)
{
pinned_image_buf buffer = *it;

env->ReleaseByteArrayElements(buffer.handle, buffer.data, JNI_ABORT);
}

env->SetLongField(thiz, ktx_buffers_field, 0);
delete buffers;
}

extern "C" JNIEXPORT jsize JNICALL Java_org_khronos_ktx_KtxTexture_getBufferListSize(JNIEnv *env, jobject thiz)
{
jclass ktx_texture_class = env->GetObjectClass(thiz);
jfieldID ktx_buffers_field = env->GetFieldID(ktx_texture_class, "buffers", "J");

std::vector<pinned_image_buf> *buffers =
reinterpret_cast<std::vector<pinned_image_buf>*>(env->GetLongField(thiz, ktx_buffers_field));

return static_cast<jsize>(buffers ? buffers->size() : 0);
}

extern "C" JNIEXPORT jboolean JNICALL Java_org_khronos_ktx_KtxTexture_isArray(JNIEnv *env, jobject thiz)
{
return get_ktx_texture(env, thiz)->isArray;
Expand Down Expand Up @@ -228,7 +151,6 @@ extern "C" JNIEXPORT void JNICALL Java_org_khronos_ktx_KtxTexture_destroy(JNIEnv
{
ktxTexture_Destroy(get_ktx_texture(env, thiz));
set_ktx_texture(env, thiz, NULL);
free_buffer_list(env, thiz);
}

extern "C" JNIEXPORT jint JNICALL Java_org_khronos_ktx_KtxTexture_setImageFromMemory(JNIEnv *env,
Expand All @@ -248,8 +170,7 @@ extern "C" JNIEXPORT jint JNICALL Java_org_khronos_ktx_KtxTexture_setImageFromMe
src,
srcSize);

push_buffer_list(env, thiz, srcArray, reinterpret_cast<jbyte*>(src));
/* DO NOT FREE SRC BUFFER NOW (see destroy()) */
env->ReleaseByteArrayElements(srcArray, reinterpret_cast<jbyte*>(src), JNI_ABORT);

return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,15 @@

public abstract class KtxTexture {
private final long instance;
private long buffers;

protected KtxTexture(long instance) {
this.instance = instance;
this.buffers = 0;
}

/* DEBUG */
public boolean isDestroyed() {
return this.instance == 0;
}
public native int getBufferListSize();
/* DEBUG */

public native boolean isArray();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright (c) 2023, Shukant Pal, robnugent, and Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.khronos.ktx.test;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.khronos.ktx.*;
import java.util.Random;
import java.util.logging.Level;
import java.util.logging.Logger;

@ExtendWith({ KtxTestLibraryLoader.class })
public class KtxParallelTest {
private static final int NUM_THREADS = 2;
private static final Logger logger = Logger.getLogger(KtxParallelTest.class.getCanonicalName());

@Test
public void testParallelAstcConversion() throws InterruptedException {
final Thread[] runThreads = new Thread[NUM_THREADS];

for (int i = 0; i < NUM_THREADS; i++) {
final KtxTestRun run = new KtxTestRun(i);
final Thread runThread = new Thread(run);
runThread.setDaemon(false);
runThread.start();

runThreads[i] = runThread;
}

for (Thread thread : runThreads) {
thread.join();
}
}

private static class KtxTestRun implements Runnable {
private final int id;
private final Random testRandomizer = new Random();

public KtxTestRun(int id) {
this.id = id;
}

public void run() {
// Repeatedly create a compress an image.
for (int i = 0; i < 300; i++) {
final int w = (testRandomizer.nextInt() % 512) + 1024;
final int h = w;
final int size = convertToASTC(w, h);

// Change level to INFO for logging
logger.log(Level.FINE,id + " iteration: " + i + ", size: " + w + "x" + h + ", compressed data size is " + size);
}
}

public int convertToASTC(int w, int h) {
// Create Uncompressed texture
final KtxTextureCreateInfo info = new KtxTextureCreateInfo();
info.setBaseWidth(w);
info.setBaseHeight(h);
info.setVkFormat(VkFormat.VK_FORMAT_R8G8B8_SRGB); // Uncompressed
final KtxTexture2 t = KtxTexture2.create(info, KtxCreateStorage.ALLOC);

// Pass the uncompressed data
int bufferSize = w * h * 3;
final byte[] rgbBA = new byte[bufferSize];
t.setImageFromMemory(0, 0, 0, rgbBA);

// Compress the data
final KtxAstcParams p = new KtxAstcParams();
p.setBlockDimension(KtxPackAstcBlockDimension.D8x8);
p.setMode(KtxPackAstcEncoderMode.LDR);
p.setQualityLevel(KtxPackAstcQualityLevel.EXHAUSTIVE);
final int rc = t.compressAstcEx(p);
if (rc != KtxErrorCode.SUCCESS) {
throw new RuntimeException("ASTC error " + rc);
}
final int retDataLen = (int) t.getDataSize();

// Free things up - segfault usually occurs inside this destroy() call
t.destroy();

return retDataLen;
}
}
}

0 comments on commit 9b084d5

Please sign in to comment.