Arrow JNI entry points leak a QueryResult on the failure path

[alvorithm]

I hit this while retrying createArrowTable in a loop against a schema the Arrow API rejects. Verified against com.ladybugdb:lbug 0.19.0, code read at ladybug-java a0e7728 (current main, and the commit pinned by ladybug v0.18.1 through v0.19.0) and ladybug 3abdb0653.

What happens

Four JNI entry points free only their own wrapper when the C API reports failure, leaving the QueryResult the out-param now owns unfreed. Measured with a C program that calls the same C API and reproduces both free patterns, 20000 failing lbug_connection_create_arrow_table calls each:

what the caller does with the out-param on failure	RSS growth
drop it, which is what the JNI does	+785 bytes/call
lbug_query_result_destroy, which is what it should do	+353 bytes/call

About 432 bytes per failed call, linear in call count and reproducible to the byte across runs at 20000 and 50000 iterations. The out-param carried a QueryResult on 20000 of 20000 failures.

Why

setQueryResult on the C API side (ladybug/src/c_api/connection.cpp:77) releases the C++ QueryResult into the out-param before it checks success, so the out-param owns it on the failure branch too:

auto queryResultPtr = queryResult.release();
outQueryResult->_query_result = queryResultPtr;
outQueryResult->_is_owned_by_cpp = false;
if (!queryResultPtr->isSuccess()) {
    return LbugError;
}

The JNI then frees only the wrapper, src/jni/lbug_java.cpp:834:

auto* queryResult = new lbug_query_result();
auto state = lbug_connection_create_arrow_table(conn, table.c_str(), schema, arrays,
    static_cast<uint64_t>(numArrays), queryResult);
if (state != LbugSuccess) {
    delete queryResult;                                  // wrapper only
    throwLastError(env, "Failed to create Arrow table");
    return jobject();
}

delete on the lbug_query_result struct does not touch _query_result. lbug_query_result_destroy is what frees it (ladybug/src/c_api/query_result.cpp:10), and it is not called.

The four sites

Every JNI entry point whose C API function routes its result through setQueryResult:

lbug_java.cpp	C API function
:835 lbugConnectionCreateArrowTable	lbug_connection_create_arrow_table
:862 lbugConnectionCreateArrowRelTable	lbug_connection_create_arrow_rel_table
:901 lbugConnectionCreateArrowRelTableCSR	lbug_connection_create_arrow_rel_table_csr
:922 lbugConnectionDropArrowTable	lbug_connection_drop_arrow_table

The fourth is reachable without any Arrow data at all:

drop_arrow_table on a missing table: state=LbugError  out._query_result=NON-NULL (owned by the out-param)
  message: Binder exception: Table no_such_table does not exist.

Not every failure leaks. When the C API returns LbugError from its catch block instead, the out-param is untouched and there is nothing to free. createArrowRelTable against a UUID-keyed node table (LadybugDB/ladybug#757) takes that route, createArrowTable with a reserved-word column name (LadybugDB/ladybug#756) takes the leaking one. Both look identical to the Java caller.

lbug_query_result_get_next_query_result does not use setQueryResult and sets _is_owned_by_cpp = true, so :1178 is fine.

Proposed fix

Call lbug_query_result_destroy(queryResult) before delete queryResult on the failure branch of all four.

The discriminator that distinguishes the two failure modes already exists in this file, in Java_com_ladybugdb_Native_lbugConnectionQuery (:717) and Java_com_ladybugdb_Native_lbugConnectionExecute (:774):

if (state != LbugSuccess && queryResult->_query_result == nullptr) {
    // Infrastructure error: no result was produced at all.
    delete queryResult;
    ...
}
// Query ran (may have failed logically): return result so Java can call
// isSuccess() / getErrorMessage().

Adopting that pattern in the four Arrow functions would fix the leak and, on the setQueryResult route, hand the failed QueryResult back to Java so the caller can read getErrorMessage(). That would also resolve the diagnosis half of this: today throwLastError falls back to its generic string, because lbug_get_last_error() is empty on exactly that route (LadybugDB/ladybug#754). Which of the two shapes you want is your call; the plain lbug_query_result_destroy fix is the smaller change if you would rather keep these functions throwing.

I have not sent this as a PR because src/jni/lbug_java.cpp has an open branch against it (fix/reject-non-value-params, for #11) and I did not want to create a conflict. Happy to open one against whatever lands.

Tests

There is no Arrow coverage under src/test/java/com/lbugdb/ yet; ConnectionTest.java is the nearest place. A case repeating a failing createArrowTable in a loop and asserting native memory does not grow is awkward to make non-flaky in JUnit, so the equivalent may sit better on the C side in ladybug's test/api/arrow_error_scenarios_test.cpp, which already covers these failure paths. Happy to put it wherever you prefer.

Reproducer

cc -o arrow_leak arrow_leak.c -I$LBUG/src/include -I$LBUG/src/include/c_api \
   -L$LBUG/build/relwithdebinfo/src -llbug -Wl,-rpath,$LBUG/build/relwithdebinfo/src
./arrow_leak jni 20000      # what the JNI does
./arrow_leak fixed 20000    # what it should do

arrow_leak.c

/* arrow_leak.c - the QueryResult that a failed createArrowTable leaves behind.
 *
 * cc -o arrow_leak arrow_leak.c -I$LBUG/src/include -I$LBUG/src/include/c_api \
 *    -L$LBUG/build/relwithdebinfo/src -llbug -Wl,-rpath,$LBUG/build/relwithdebinfo/src
 *
 * lbug_connection_create_arrow_table returns LbugError through setQueryResult, which has
 * already released the C++ QueryResult into the out-param. Whoever gets that out-param owns
 * it. tools/java_api/src/jni/lbug_java.cpp:834 frees only its own wrapper on this path and
 * never calls lbug_query_result_destroy, so the QueryResult is never freed.
 *
 * Mode "jni"  reproduces that: drop the out-param without destroying it.
 * Mode "fixed" calls lbug_query_result_destroy, which is what the JNI should do.
 */
#include <lbug.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

static long rss_kb(void) {
    FILE* f = fopen("/proc/self/status", "r");
    if (!f)
        return -1;
    char line[256];
    long kb = -1;
    while (fgets(line, sizeof line, f))
        if (!strncmp(line, "VmRSS:", 6)) {
            sscanf(line + 6, "%ld", &kb);
            break;
        }
    fclose(f);
    return kb;
}

/* ---- minimal Arrow C Data Interface producer: one row, columns "id" and "index" ---- */

static void release_schema(struct ArrowSchema* s) {
    for (int64_t i = 0; i < s->n_children; i++)
        if (s->children[i]->release)
            s->children[i]->release(s->children[i]);
    free(s->children);
    s->release = NULL;
}

static void release_array(struct ArrowArray* a) {
    for (int64_t i = 0; i < a->n_children; i++)
        if (a->children[i]->release)
            a->children[i]->release(a->children[i]);
    free(a->children);
    free((void*)a->buffers);
    a->release = NULL;
}

static void init_schema(struct ArrowSchema* s, const char* fmt, const char* name) {
    memset(s, 0, sizeof *s);
    s->format = fmt;
    s->name = name;
    s->release = release_schema;
}

static int32_t g_offs[2] = {0, 2};

static void utf8_array(struct ArrowArray* a, const char* value) {
    memset(a, 0, sizeof *a);
    a->length = 1;
    a->null_count = 0;
    a->n_buffers = 3;
    const void** bufs = calloc(3, sizeof(void*));
    bufs[0] = NULL;
    bufs[1] = g_offs;
    bufs[2] = (void*)value;
    a->buffers = bufs;
    a->release = release_array;
}

static void build(struct ArrowSchema* schema, struct ArrowArray* array) {
    init_schema(schema, "+s", NULL);
    schema->n_children = 2;
    schema->children = calloc(2, sizeof(struct ArrowSchema*));
    const char* names[2] = {"id", "index"}; /* "index" makes the generated DDL invalid */
    for (int i = 0; i < 2; i++) {
        schema->children[i] = calloc(1, sizeof(struct ArrowSchema));
        init_schema(schema->children[i], "u", names[i]);
    }
    memset(array, 0, sizeof *array);
    array->length = 1;
    array->n_buffers = 1;
    const void** rb = calloc(1, sizeof(void*));
    rb[0] = NULL;
    array->buffers = rb;
    array->n_children = 2;
    array->children = calloc(2, sizeof(struct ArrowArray*));
    for (int i = 0; i < 2; i++) {
        array->children[i] = calloc(1, sizeof(struct ArrowArray));
        utf8_array(array->children[i], "r1");
    }
    array->release = release_array;
}

int main(int argc, char** argv) {
    const char* mode = argc > 1 ? argv[1] : "jni";
    int n = argc > 2 ? atoi(argv[2]) : 20000;
    int destroy = !strcmp(mode, "fixed");

    lbug_database db;
    lbug_connection conn;
    if (lbug_database_init("", lbug_default_system_config(), &db) != LbugSuccess) {
        fprintf(stderr, "database_init failed\n");
        return 1;
    }
    lbug_connection_init(&db, &conn);

    /* warm up so the first measurement does not include allocator growth */
    for (int i = 0; i < 500; i++) {
        struct ArrowSchema s;
        struct ArrowArray a;
        build(&s, &a);
        lbug_query_result out;
        memset(&out, 0, sizeof out);
        lbug_connection_create_arrow_table(&conn, "warm", &s, &a, 1, &out);
        lbug_query_result_destroy(&out);
    }

    long before = rss_kb();
    long nonNullOutParams = 0;
    for (int i = 0; i < n; i++) {
        struct ArrowSchema s;
        struct ArrowArray a;
        build(&s, &a);
        lbug_query_result out;
        memset(&out, 0, sizeof out);
        lbug_state st = lbug_connection_create_arrow_table(&conn, "t", &s, &a, 1, &out);
        if (st == LbugSuccess) {
            fprintf(stderr, "unexpected success\n");
            return 1;
        }
        if (out._query_result != NULL)
            nonNullOutParams++;
        if (destroy)
            lbug_query_result_destroy(&out); /* what the JNI should do */
        /* else: drop it, which is what the JNI does */
    }
    long after = rss_kb();

    printf("mode=%-5s iterations=%d  RSS %ld -> %ld kB  (%+.0f bytes/iteration)\n", mode, n, before,
        after, (after - before) * 1024.0 / n);
    printf("  out-param carried a QueryResult on %ld of %d failures\n", nonNullOutParams, n);

    lbug_connection_destroy(&conn);
    lbug_database_destroy(&db);
    return 0;
}

Notes

Verified against com.ladybugdb:lbug 0.19.0 on Linux. src/jni/lbug_java.cpp at a0e7728 is unchanged since v0.17.0 and is the commit every release from v0.18.1 to v0.19.0 pins. Measurements are RSS from /proc/self/status, in a C process with no JVM involved, warmed up before measuring.