From 17b71c69e2fdd489e9175e2bb1bc935ff57d9d73 Mon Sep 17 00:00:00 2001 From: dhoard Date: Sun, 12 Jul 2026 21:52:34 -0400 Subject: [PATCH 1/7] fix: Fixed commit hook Signed-off-by: dhoard --- src/main/c/com_tidesdb_TidesDB.c | 402 ++++++++++++--------- src/test/java/com/tidesdb/TidesDBTest.java | 316 ++++++++++++++++ 2 files changed, 541 insertions(+), 177 deletions(-) diff --git a/src/main/c/com_tidesdb_TidesDB.c b/src/main/c/com_tidesdb_TidesDB.c index 7418337..122c455 100644 --- a/src/main/c/com_tidesdb_TidesDB.c +++ b/src/main/c/com_tidesdb_TidesDB.c @@ -17,6 +17,7 @@ * limitations under the License. */ #include +#include #include #include #include @@ -219,10 +220,9 @@ JNIEXPORT jlong JNICALL Java_com_tidesdb_TidesDB_nativeObjstoreS3Create( tdb_s3_create_config_fn create_fn = resolve_s3_create_config(); if (create_fn == NULL) { - throwTidesDBException( - env, TDB_ERR_INVALID_ARGS, - "TidesDB was built without S3 support (rebuild the core library with " - "TIDESDB_WITH_S3=ON)"); + throwTidesDBException(env, TDB_ERR_INVALID_ARGS, + "TidesDB was built without S3 support (rebuild the core library with " + "TIDESDB_WITH_S3=ON)"); return 0; } @@ -236,19 +236,18 @@ JNIEXPORT jlong JNICALL Java_com_tidesdb_TidesDB_nativeObjstoreS3Create( const char *c_region = region ? (*env)->GetStringUTFChars(env, region, NULL) : NULL; const char *c_ca = tlsCaPath ? (*env)->GetStringUTFChars(env, tlsCaPath, NULL) : NULL; - tidesdb_objstore_s3_config_t cfg = { - .endpoint = c_endpoint, - .bucket = c_bucket, - .prefix = c_prefix, - .access_key = c_access, - .secret_key = c_secret, - .region = c_region, - .use_ssl = useSsl ? 1 : 0, - .use_path_style = usePathStyle ? 1 : 0, - .tls_ca_path = c_ca, - .tls_insecure_skip_verify = tlsInsecureSkipVerify ? 1 : 0, - .multipart_threshold = (size_t)multipartThreshold, - .multipart_part_size = (size_t)multipartPartSize}; + tidesdb_objstore_s3_config_t cfg = {.endpoint = c_endpoint, + .bucket = c_bucket, + .prefix = c_prefix, + .access_key = c_access, + .secret_key = c_secret, + .region = c_region, + .use_ssl = useSsl ? 1 : 0, + .use_path_style = usePathStyle ? 1 : 0, + .tls_ca_path = c_ca, + .tls_insecure_skip_verify = tlsInsecureSkipVerify ? 1 : 0, + .multipart_threshold = (size_t)multipartThreshold, + .multipart_part_size = (size_t)multipartPartSize}; tidesdb_objstore_t *connector = create_fn(&cfg); @@ -287,9 +286,8 @@ JNIEXPORT void JNICALL Java_com_tidesdb_TidesDB_nativeCreateColumnFamily( jboolean enableBlockIndexes, jint indexSampleRatio, jint blockIndexPrefixLen, jint syncMode, jlong syncIntervalUs, jstring comparatorName, jint skipListMaxLevel, jfloat skipListProbability, jint defaultIsolationLevel, jlong minDiskSpace, jint l1FileCountTrigger, - jint l0QueueStallThreshold, jdouble tombstoneDensityTrigger, - jlong tombstoneDensityMinEntries, jboolean useBtree, - jboolean objectLazyCompaction, jboolean objectPrefetchCompaction) + jint l0QueueStallThreshold, jdouble tombstoneDensityTrigger, jlong tombstoneDensityMinEntries, + jboolean useBtree, jboolean objectLazyCompaction, jboolean objectPrefetchCompaction) { tidesdb_t *db = (tidesdb_t *)(uintptr_t)handle; const char *cfName = (*env)->GetStringUTFChars(env, name, NULL); @@ -559,9 +557,9 @@ JNIEXPORT void JNICALL Java_com_tidesdb_TidesDB_nativeCheckpoint(JNIEnv *env, jc } JNIEXPORT void JNICALL Java_com_tidesdb_TidesDB_nativeRenameColumnFamily(JNIEnv *env, jclass cls, - jlong handle, - jstring oldName, - jstring newName) + jlong handle, + jstring oldName, + jstring newName) { tidesdb_t *db = (tidesdb_t *)(uintptr_t)handle; const char *oldCfName = (*env)->GetStringUTFChars(env, oldName, NULL); @@ -695,25 +693,17 @@ JNIEXPORT jobject JNICALL Java_com_tidesdb_ColumnFamily_nativeGetStats(JNIEnv *e jstring comparatorName = (*env)->NewStringUTF(env, stats->config->comparator_name); cfConfigObj = (*env)->CallStaticObjectMethod( - env, cfConfigClass, fromNative, - (jlong)stats->config->write_buffer_size, - (jlong)stats->config->level_size_ratio, - (jint)stats->config->min_levels, - (jint)stats->config->dividing_level_offset, - (jlong)stats->config->klog_value_threshold, + env, cfConfigClass, fromNative, (jlong)stats->config->write_buffer_size, + (jlong)stats->config->level_size_ratio, (jint)stats->config->min_levels, + (jint)stats->config->dividing_level_offset, (jlong)stats->config->klog_value_threshold, (jint)stats->config->compression_algorithm, stats->config->enable_bloom_filter != 0 ? JNI_TRUE : JNI_FALSE, (jdouble)stats->config->bloom_fpr, stats->config->enable_block_indexes != 0 ? JNI_TRUE : JNI_FALSE, - (jint)stats->config->index_sample_ratio, - (jint)stats->config->block_index_prefix_len, - (jint)stats->config->sync_mode, - (jlong)stats->config->sync_interval_us, - comparatorName, - (jint)stats->config->skip_list_max_level, - (jfloat)stats->config->skip_list_probability, - (jint)stats->config->default_isolation_level, - (jlong)stats->config->min_disk_space, + (jint)stats->config->index_sample_ratio, (jint)stats->config->block_index_prefix_len, + (jint)stats->config->sync_mode, (jlong)stats->config->sync_interval_us, comparatorName, + (jint)stats->config->skip_list_max_level, (jfloat)stats->config->skip_list_probability, + (jint)stats->config->default_isolation_level, (jlong)stats->config->min_disk_space, (jint)stats->config->l1_file_count_trigger, (jint)stats->config->l0_queue_stall_threshold, (jdouble)stats->config->tombstone_density_trigger, @@ -727,29 +717,21 @@ JNIEXPORT jobject JNICALL Java_com_tidesdb_ColumnFamily_nativeGetStats(JNIEnv *e } jclass statsClass = (*env)->FindClass(env, "com/tidesdb/Stats"); - jmethodID constructor = (*env)->GetMethodID(env, statsClass, "", - "(IJ[J[ILcom/tidesdb/ColumnFamilyConfig;JJDD[JDDZJIDJD[JDIJJJJJJJ)V"); - - jobject statsObj = (*env)->NewObject(env, statsClass, constructor, stats->num_levels, - (jlong)stats->memtable_size, levelSizes, levelNumSSTables, - cfConfigObj, (jlong)stats->total_keys, - (jlong)stats->total_data_size, - stats->avg_key_size, stats->avg_value_size, levelKeyCounts, - stats->read_amp, stats->hit_rate, - stats->use_btree != 0, (jlong)stats->btree_total_nodes, - (jint)stats->btree_max_height, stats->btree_avg_height, - (jlong)stats->total_tombstones, - (jdouble)stats->tombstone_ratio, - levelTombstoneCounts, - (jdouble)stats->max_sst_density, - (jint)stats->max_sst_density_level, - (jlong)stats->wal_bytes_written, - (jlong)stats->flush_bytes_written, - (jlong)stats->compaction_bytes_written, - (jlong)stats->compaction_bytes_read, - (jlong)stats->user_bytes_written, - (jlong)stats->flush_count, - (jlong)stats->compaction_count); + jmethodID constructor = + (*env)->GetMethodID(env, statsClass, "", + "(IJ[J[ILcom/tidesdb/ColumnFamilyConfig;JJDD[JDDZJIDJD[JDIJJJJJJJ)V"); + + jobject statsObj = (*env)->NewObject( + env, statsClass, constructor, stats->num_levels, (jlong)stats->memtable_size, levelSizes, + levelNumSSTables, cfConfigObj, (jlong)stats->total_keys, (jlong)stats->total_data_size, + stats->avg_key_size, stats->avg_value_size, levelKeyCounts, stats->read_amp, + stats->hit_rate, stats->use_btree != 0, (jlong)stats->btree_total_nodes, + (jint)stats->btree_max_height, stats->btree_avg_height, (jlong)stats->total_tombstones, + (jdouble)stats->tombstone_ratio, levelTombstoneCounts, (jdouble)stats->max_sst_density, + (jint)stats->max_sst_density_level, (jlong)stats->wal_bytes_written, + (jlong)stats->flush_bytes_written, (jlong)stats->compaction_bytes_written, + (jlong)stats->compaction_bytes_read, (jlong)stats->user_bytes_written, + (jlong)stats->flush_count, (jlong)stats->compaction_count); tidesdb_free_stats(stats); @@ -942,8 +924,7 @@ JNIEXPORT void JNICALL Java_com_tidesdb_Transaction_nativeDelete(JNIEnv *env, jc } JNIEXPORT void JNICALL Java_com_tidesdb_Transaction_nativeSingleDelete(JNIEnv *env, jclass cls, - jlong handle, - jlong cfHandle, + jlong handle, jlong cfHandle, jbyteArray key) { tidesdb_txn_t *txn = (tidesdb_txn_t *)(uintptr_t)handle; @@ -1221,33 +1202,82 @@ JNIEXPORT void JNICALL Java_com_tidesdb_TidesDBIterator_nativeFree(JNIEnv *env, /** * Context stored as the commit hook ctx pointer. * Holds the JavaVM and a global reference to the Java CommitHook object. + * Uses reference-counted quiescent retirement to avoid use-after-free. */ typedef struct { JavaVM *jvm; jobject hook_obj; /* global reference to CommitHook */ + int refcount; /* callback reference count */ + int retired; /* 0 = active, 1 = retired (do not enter) */ + pthread_mutex_t lock; + pthread_cond_t zero_cond; } java_hook_ctx_t; +/** + * Retires and destroys a hook context. Waits for in-flight callbacks to drain, + * then deletes the global reference and frees the context. + * Must only be called after the context has been detached from tidesdb (i.e., + * after tidesdb_cf_set_commit_hook has replaced it). + */ +static void retire_and_destroy_hook_ctx(JNIEnv *env, java_hook_ctx_t *ctx) +{ + if (ctx == NULL) return; + + pthread_mutex_lock(&ctx->lock); + ctx->retired = 1; + while (ctx->refcount > 0) + { + pthread_cond_wait(&ctx->zero_cond, &ctx->lock); + } + pthread_mutex_unlock(&ctx->lock); + + (*env)->DeleteGlobalRef(env, ctx->hook_obj); + pthread_mutex_destroy(&ctx->lock); + pthread_cond_destroy(&ctx->zero_cond); + free(ctx); +} + /** * C trampoline that bridges the tidesdb_commit_hook_fn callback to the Java CommitHook.onCommit * method. Fires synchronously on the committing thread (which is always a Java thread). */ static int java_commit_hook_trampoline(const tidesdb_commit_op_t *ops, int num_ops, - uint64_t commit_seq, void *ctx) + uint64_t commit_seq, void *ctx) { java_hook_ctx_t *hctx = (java_hook_ctx_t *)ctx; JNIEnv *env = NULL; int need_detach = 0; + /* Check if context is retired under lock, increment refcount if active */ + pthread_mutex_lock(&hctx->lock); + if (hctx->retired) + { + pthread_mutex_unlock(&hctx->lock); + return -1; + } + hctx->refcount++; + pthread_mutex_unlock(&hctx->lock); + jint rc = (*hctx->jvm)->GetEnv(hctx->jvm, (void **)&env, JNI_VERSION_1_6); if (rc == JNI_EDETACHED) { if ((*hctx->jvm)->AttachCurrentThread(hctx->jvm, (void **)&env, NULL) != 0) + { + pthread_mutex_lock(&hctx->lock); + hctx->refcount--; + if (hctx->refcount == 0 && hctx->retired) pthread_cond_signal(&hctx->zero_cond); + pthread_mutex_unlock(&hctx->lock); return -1; + } need_detach = 1; } else if (rc != JNI_OK) { + pthread_mutex_lock(&hctx->lock); + hctx->refcount--; + if (hctx->refcount == 0 && hctx->retired) pthread_cond_signal(&hctx->zero_cond); + pthread_mutex_unlock(&hctx->lock); return -1; } @@ -1255,17 +1285,14 @@ static int java_commit_hook_trampoline(const tidesdb_commit_op_t *ops, int num_o /* Find CommitOp class and constructor: CommitOp(byte[], byte[], long, boolean) */ jclass commitOpClass = (*env)->FindClass(env, "com/tidesdb/CommitOp"); - if (commitOpClass == NULL) - goto cleanup; + if (commitOpClass == NULL) goto cleanup; jmethodID ctor = (*env)->GetMethodID(env, commitOpClass, "", "([B[BJZ)V"); - if (ctor == NULL) - goto cleanup; + if (ctor == NULL) goto cleanup; /* Create CommitOp[] array */ jobjectArray opsArray = (*env)->NewObjectArray(env, num_ops, commitOpClass, NULL); - if (opsArray == NULL) - goto cleanup; + if (opsArray == NULL) goto cleanup; for (int i = 0; i < num_ops; i++) { @@ -1280,15 +1307,13 @@ static int java_commit_hook_trampoline(const tidesdb_commit_op_t *ops, int num_o (jbyte *)ops[i].value); } - jobject opObj = (*env)->NewObject(env, commitOpClass, ctor, jkey, jvalue, - (jlong)ops[i].ttl, + jobject opObj = (*env)->NewObject(env, commitOpClass, ctor, jkey, jvalue, (jlong)ops[i].ttl, ops[i].is_delete ? JNI_TRUE : JNI_FALSE); (*env)->SetObjectArrayElement(env, opsArray, i, opObj); (*env)->DeleteLocalRef(env, opObj); (*env)->DeleteLocalRef(env, jkey); - if (jvalue != NULL) - (*env)->DeleteLocalRef(env, jvalue); + if (jvalue != NULL) (*env)->DeleteLocalRef(env, jvalue); } /* Call CommitHook.onCommit(CommitOp[], long) */ @@ -1308,34 +1333,36 @@ static int java_commit_hook_trampoline(const tidesdb_commit_op_t *ops, int num_o (*env)->DeleteLocalRef(env, commitOpClass); (*env)->DeleteLocalRef(env, hookClass); - if (need_detach) - (*hctx->jvm)->DetachCurrentThread(hctx->jvm); + if (need_detach) (*hctx->jvm)->DetachCurrentThread(hctx->jvm); + + /* Decrement refcount and signal if retiring */ + pthread_mutex_lock(&hctx->lock); + hctx->refcount--; + if (hctx->refcount == 0 && hctx->retired) pthread_cond_signal(&hctx->zero_cond); + pthread_mutex_unlock(&hctx->lock); return (int)ret; cleanup: - if ((*env)->ExceptionCheck(env)) - (*env)->ExceptionClear(env); - if (need_detach) - (*hctx->jvm)->DetachCurrentThread(hctx->jvm); + if ((*env)->ExceptionCheck(env)) (*env)->ExceptionClear(env); + if (need_detach) (*hctx->jvm)->DetachCurrentThread(hctx->jvm); + + /* Decrement refcount and signal if retiring */ + pthread_mutex_lock(&hctx->lock); + hctx->refcount--; + if (hctx->refcount == 0 && hctx->retired) pthread_cond_signal(&hctx->zero_cond); + pthread_mutex_unlock(&hctx->lock); + return -1; } JNIEXPORT jlong JNICALL Java_com_tidesdb_ColumnFamily_nativeSetCommitHook(JNIEnv *env, jclass cls, - jlong cfHandle, - jobject hook, - jlong oldCtxHandle) + jlong cfHandle, + jobject hook, + jlong oldCtxHandle) { tidesdb_column_family_t *cf = (tidesdb_column_family_t *)(uintptr_t)cfHandle; - /* Free old context if present */ - if (oldCtxHandle != 0) - { - java_hook_ctx_t *old_ctx = (java_hook_ctx_t *)(uintptr_t)oldCtxHandle; - (*env)->DeleteGlobalRef(env, old_ctx->hook_obj); - free(old_ctx); - } - /* If hook is NULL, clear the hook */ if (hook == NULL) { @@ -1343,35 +1370,77 @@ JNIEXPORT jlong JNICALL Java_com_tidesdb_ColumnFamily_nativeSetCommitHook(JNIEnv if (result != TDB_SUCCESS) { throwTidesDBException(env, result, getErrorMessage(result)); + return oldCtxHandle; /* old context remains active */ } + + /* Retire old context after successful detachment */ + if (oldCtxHandle != 0) + { + java_hook_ctx_t *old_ctx = (java_hook_ctx_t *)(uintptr_t)oldCtxHandle; + retire_and_destroy_hook_ctx(env, old_ctx); + } + return 0; } /* Allocate new context */ - java_hook_ctx_t *ctx = (java_hook_ctx_t *)malloc(sizeof(java_hook_ctx_t)); - if (ctx == NULL) + java_hook_ctx_t *new_ctx = (java_hook_ctx_t *)malloc(sizeof(java_hook_ctx_t)); + if (new_ctx == NULL) { throwTidesDBException(env, TDB_ERR_MEMORY, "Failed to allocate commit hook context"); - return 0; + return oldCtxHandle; /* old context remains active */ } - (*env)->GetJavaVM(env, &ctx->jvm); - ctx->hook_obj = (*env)->NewGlobalRef(env, hook); + /* Initialize refcount and retired flag */ + new_ctx->refcount = 0; + new_ctx->retired = 0; + pthread_mutex_init(&new_ctx->lock, NULL); + pthread_cond_init(&new_ctx->zero_cond, NULL); - int result = tidesdb_cf_set_commit_hook(cf, java_commit_hook_trampoline, ctx); + if ((*env)->GetJavaVM(env, &new_ctx->jvm) != 0) + { + pthread_mutex_destroy(&new_ctx->lock); + pthread_cond_destroy(&new_ctx->zero_cond); + free(new_ctx); + throwTidesDBException(env, TDB_ERR_MEMORY, "Failed to get JavaVM"); + return oldCtxHandle; + } + + new_ctx->hook_obj = (*env)->NewGlobalRef(env, hook); + if (new_ctx->hook_obj == NULL) + { + if ((*env)->ExceptionCheck(env)) (*env)->ExceptionClear(env); + pthread_mutex_destroy(&new_ctx->lock); + pthread_cond_destroy(&new_ctx->zero_cond); + free(new_ctx); + throwTidesDBException(env, TDB_ERR_MEMORY, + "Failed to create global reference for commit hook"); + return oldCtxHandle; + } + + int result = tidesdb_cf_set_commit_hook(cf, java_commit_hook_trampoline, new_ctx); if (result != TDB_SUCCESS) { - (*env)->DeleteGlobalRef(env, ctx->hook_obj); - free(ctx); + (*env)->DeleteGlobalRef(env, new_ctx->hook_obj); + pthread_mutex_destroy(&new_ctx->lock); + pthread_cond_destroy(&new_ctx->zero_cond); + free(new_ctx); throwTidesDBException(env, result, getErrorMessage(result)); - return 0; + return oldCtxHandle; /* old context remains active */ } - return (jlong)(uintptr_t)ctx; + /* Retire old context after successful hook replacement */ + if (oldCtxHandle != 0) + { + java_hook_ctx_t *old_ctx = (java_hook_ctx_t *)(uintptr_t)oldCtxHandle; + retire_and_destroy_hook_ctx(env, old_ctx); + } + + return (jlong)(uintptr_t)new_ctx; } JNIEXPORT void JNICALL Java_com_tidesdb_ColumnFamily_nativePurge(JNIEnv *env, jclass cls, - jlong handle) + jlong handle) { tidesdb_column_family_t *cf = (tidesdb_column_family_t *)(uintptr_t)handle; int result = tidesdb_purge_cf(cf); @@ -1383,7 +1452,7 @@ JNIEXPORT void JNICALL Java_com_tidesdb_ColumnFamily_nativePurge(JNIEnv *env, jc } JNIEXPORT void JNICALL Java_com_tidesdb_ColumnFamily_nativeSyncWal(JNIEnv *env, jclass cls, - jlong handle) + jlong handle) { tidesdb_column_family_t *cf = (tidesdb_column_family_t *)(uintptr_t)handle; int result = tidesdb_sync_wal(cf); @@ -1406,7 +1475,7 @@ JNIEXPORT void JNICALL Java_com_tidesdb_TidesDB_nativePurge(JNIEnv *env, jclass } JNIEXPORT jobject JNICALL Java_com_tidesdb_TidesDB_nativeGetDbStats(JNIEnv *env, jclass cls, - jlong handle) + jlong handle) { tidesdb_t *db = (tidesdb_t *)(uintptr_t)handle; tidesdb_db_stats_t db_stats; @@ -1426,9 +1495,9 @@ JNIEXPORT jobject JNICALL Java_com_tidesdb_TidesDB_nativeGetDbStats(JNIEnv *env, boolean, String, long, long, int, long, long, long, long, boolean, long (primaryEpoch), long (seenEpoch), long, long, long, long, long, long, long, long */ - jmethodID constructor = (*env)->GetMethodID( - env, dbStatsClass, "", - "(IJJJIIJIIJIJJJJZJIZIJZLjava/lang/String;JJIJJJJZJJJJJJJJJJ)V"); + jmethodID constructor = + (*env)->GetMethodID(env, dbStatsClass, "", + "(IJJJIIJIIJIJJJJZJIZIJZLjava/lang/String;JJIJJJJZJJJJJJJJJJ)V"); jstring connectorStr = NULL; if (db_stats.object_store_connector != NULL) @@ -1436,54 +1505,33 @@ JNIEXPORT jobject JNICALL Java_com_tidesdb_TidesDB_nativeGetDbStats(JNIEnv *env, connectorStr = (*env)->NewStringUTF(env, db_stats.object_store_connector); } - return (*env)->NewObject(env, dbStatsClass, constructor, - (jint)db_stats.num_column_families, - (jlong)db_stats.total_memory, - (jlong)db_stats.available_memory, - (jlong)db_stats.resolved_memory_limit, - (jint)db_stats.memory_pressure_level, - (jint)db_stats.flush_pending_count, - (jlong)db_stats.total_memtable_bytes, - (jint)db_stats.total_immutable_count, - (jint)db_stats.total_sstable_count, - (jlong)db_stats.total_data_size_bytes, - (jint)db_stats.num_open_sstables, - (jlong)db_stats.global_seq, - (jlong)db_stats.txn_memory_bytes, - (jlong)db_stats.compaction_queue_size, - (jlong)db_stats.flush_queue_size, - db_stats.unified_memtable_enabled != 0, - (jlong)db_stats.unified_memtable_bytes, - (jint)db_stats.unified_immutable_count, - db_stats.unified_is_flushing != 0, - (jint)db_stats.unified_next_cf_index, - (jlong)db_stats.unified_wal_generation, - db_stats.object_store_enabled != 0, - connectorStr, - (jlong)db_stats.local_cache_bytes_used, - (jlong)db_stats.local_cache_bytes_max, - (jint)db_stats.local_cache_num_files, - (jlong)db_stats.last_uploaded_generation, - (jlong)db_stats.upload_queue_depth, - (jlong)db_stats.total_uploads, - (jlong)db_stats.total_upload_failures, - db_stats.replica_mode != 0, - (jlong)db_stats.primary_epoch, - (jlong)db_stats.seen_epoch, - (jlong)db_stats.uwal_bytes_written, - (jlong)db_stats.wal_bytes_written, - (jlong)db_stats.flush_bytes_written, - (jlong)db_stats.compaction_bytes_written, - (jlong)db_stats.compaction_bytes_read, - (jlong)db_stats.user_bytes_written, - (jlong)db_stats.flush_count, - (jlong)db_stats.compaction_count); + return (*env)->NewObject( + env, dbStatsClass, constructor, (jint)db_stats.num_column_families, + (jlong)db_stats.total_memory, (jlong)db_stats.available_memory, + (jlong)db_stats.resolved_memory_limit, (jint)db_stats.memory_pressure_level, + (jint)db_stats.flush_pending_count, (jlong)db_stats.total_memtable_bytes, + (jint)db_stats.total_immutable_count, (jint)db_stats.total_sstable_count, + (jlong)db_stats.total_data_size_bytes, (jint)db_stats.num_open_sstables, + (jlong)db_stats.global_seq, (jlong)db_stats.txn_memory_bytes, + (jlong)db_stats.compaction_queue_size, (jlong)db_stats.flush_queue_size, + db_stats.unified_memtable_enabled != 0, (jlong)db_stats.unified_memtable_bytes, + (jint)db_stats.unified_immutable_count, db_stats.unified_is_flushing != 0, + (jint)db_stats.unified_next_cf_index, (jlong)db_stats.unified_wal_generation, + db_stats.object_store_enabled != 0, connectorStr, (jlong)db_stats.local_cache_bytes_used, + (jlong)db_stats.local_cache_bytes_max, (jint)db_stats.local_cache_num_files, + (jlong)db_stats.last_uploaded_generation, (jlong)db_stats.upload_queue_depth, + (jlong)db_stats.total_uploads, (jlong)db_stats.total_upload_failures, + db_stats.replica_mode != 0, (jlong)db_stats.primary_epoch, (jlong)db_stats.seen_epoch, + (jlong)db_stats.uwal_bytes_written, (jlong)db_stats.wal_bytes_written, + (jlong)db_stats.flush_bytes_written, (jlong)db_stats.compaction_bytes_written, + (jlong)db_stats.compaction_bytes_read, (jlong)db_stats.user_bytes_written, + (jlong)db_stats.flush_count, (jlong)db_stats.compaction_count); } JNIEXPORT jdouble JNICALL Java_com_tidesdb_ColumnFamily_nativeRangeCost(JNIEnv *env, jclass cls, - jlong handle, - jbyteArray keyA, - jbyteArray keyB) + jlong handle, + jbyteArray keyA, + jbyteArray keyB) { tidesdb_column_family_t *cf = (tidesdb_column_family_t *)(uintptr_t)handle; @@ -1494,8 +1542,8 @@ JNIEXPORT jdouble JNICALL Java_com_tidesdb_ColumnFamily_nativeRangeCost(JNIEnv * jbyte *keyBBytes = (*env)->GetByteArrayElements(env, keyB, NULL); double cost = 0.0; - int result = tidesdb_range_cost(cf, (uint8_t *)keyABytes, keyALen, (uint8_t *)keyBBytes, - keyBLen, &cost); + int result = + tidesdb_range_cost(cf, (uint8_t *)keyABytes, keyALen, (uint8_t *)keyBBytes, keyBLen, &cost); (*env)->ReleaseByteArrayElements(env, keyA, keyABytes, JNI_ABORT); (*env)->ReleaseByteArrayElements(env, keyB, keyBBytes, JNI_ABORT); @@ -1510,8 +1558,8 @@ JNIEXPORT jdouble JNICALL Java_com_tidesdb_ColumnFamily_nativeRangeCost(JNIEnv * } JNIEXPORT void JNICALL Java_com_tidesdb_TidesDB_nativeDeleteColumnFamily(JNIEnv *env, jclass cls, - jlong handle, - jlong cfHandle) + jlong handle, + jlong cfHandle) { tidesdb_t *db = (tidesdb_t *)(uintptr_t)handle; tidesdb_column_family_t *cf = (tidesdb_column_family_t *)(uintptr_t)cfHandle; @@ -1525,7 +1573,7 @@ JNIEXPORT void JNICALL Java_com_tidesdb_TidesDB_nativeDeleteColumnFamily(JNIEnv } JNIEXPORT void JNICALL Java_com_tidesdb_TidesDB_nativePromoteToPrimary(JNIEnv *env, jclass cls, - jlong handle) + jlong handle) { tidesdb_t *db = (tidesdb_t *)(uintptr_t)handle; @@ -1557,21 +1605,21 @@ JNIEXPORT jlong JNICALL Java_com_tidesdb_TidesDB_nativeRaiseOpenFileLimit(JNIEnv } JNIEXPORT jint JNICALL Java_com_tidesdb_Config_nativeDefaultMaxConcurrentFlushes(JNIEnv *env, - jclass cls) + jclass cls) { tidesdb_config_t cfg = tidesdb_default_config(); return (jint)cfg.max_concurrent_flushes; } -JNIEXPORT jdouble JNICALL Java_com_tidesdb_ColumnFamilyConfig_nativeDefaultTombstoneDensityTrigger( - JNIEnv *env, jclass cls) +JNIEXPORT jdouble JNICALL +Java_com_tidesdb_ColumnFamilyConfig_nativeDefaultTombstoneDensityTrigger(JNIEnv *env, jclass cls) { tidesdb_column_family_config_t cfg = tidesdb_default_column_family_config(); return (jdouble)cfg.tombstone_density_trigger; } -JNIEXPORT jlong JNICALL Java_com_tidesdb_ColumnFamilyConfig_nativeDefaultTombstoneDensityMinEntries( - JNIEnv *env, jclass cls) +JNIEXPORT jlong JNICALL +Java_com_tidesdb_ColumnFamilyConfig_nativeDefaultTombstoneDensityMinEntries(JNIEnv *env, jclass cls) { tidesdb_column_family_config_t cfg = tidesdb_default_column_family_config(); return (jlong)cfg.tombstone_density_min_entries; @@ -1598,18 +1646,16 @@ static jobject buildCfConfigObject(JNIEnv *env, const tidesdb_column_family_conf jstring comparatorName = (*env)->NewStringUTF(env, cfg->comparator_name); jobject obj = (*env)->CallStaticObjectMethod( - env, cfConfigClass, fromNative, - (jlong)cfg->write_buffer_size, (jlong)cfg->level_size_ratio, (jint)cfg->min_levels, - (jint)cfg->dividing_level_offset, (jlong)cfg->klog_value_threshold, - (jint)cfg->compression_algorithm, - cfg->enable_bloom_filter != 0 ? JNI_TRUE : JNI_FALSE, (jdouble)cfg->bloom_fpr, - cfg->enable_block_indexes != 0 ? JNI_TRUE : JNI_FALSE, (jint)cfg->index_sample_ratio, - (jint)cfg->block_index_prefix_len, (jint)cfg->sync_mode, (jlong)cfg->sync_interval_us, - comparatorName, (jint)cfg->skip_list_max_level, (jfloat)cfg->skip_list_probability, - (jint)cfg->default_isolation_level, (jlong)cfg->min_disk_space, - (jint)cfg->l1_file_count_trigger, (jint)cfg->l0_queue_stall_threshold, - (jdouble)cfg->tombstone_density_trigger, (jlong)cfg->tombstone_density_min_entries, - cfg->use_btree != 0 ? JNI_TRUE : JNI_FALSE, + env, cfConfigClass, fromNative, (jlong)cfg->write_buffer_size, (jlong)cfg->level_size_ratio, + (jint)cfg->min_levels, (jint)cfg->dividing_level_offset, (jlong)cfg->klog_value_threshold, + (jint)cfg->compression_algorithm, cfg->enable_bloom_filter != 0 ? JNI_TRUE : JNI_FALSE, + (jdouble)cfg->bloom_fpr, cfg->enable_block_indexes != 0 ? JNI_TRUE : JNI_FALSE, + (jint)cfg->index_sample_ratio, (jint)cfg->block_index_prefix_len, (jint)cfg->sync_mode, + (jlong)cfg->sync_interval_us, comparatorName, (jint)cfg->skip_list_max_level, + (jfloat)cfg->skip_list_probability, (jint)cfg->default_isolation_level, + (jlong)cfg->min_disk_space, (jint)cfg->l1_file_count_trigger, + (jint)cfg->l0_queue_stall_threshold, (jdouble)cfg->tombstone_density_trigger, + (jlong)cfg->tombstone_density_min_entries, cfg->use_btree != 0 ? JNI_TRUE : JNI_FALSE, cfg->object_lazy_compaction != 0 ? JNI_TRUE : JNI_FALSE, cfg->object_prefetch_compaction != 0 ? JNI_TRUE : JNI_FALSE); @@ -1696,8 +1742,10 @@ JNIEXPORT void JNICALL Java_com_tidesdb_ColumnFamilyConfig_nativeSaveToIni( } } -JNIEXPORT jobject JNICALL Java_com_tidesdb_ColumnFamilyConfig_nativeLoadFromIni( - JNIEnv *env, jclass cls, jstring iniFile, jstring sectionName) +JNIEXPORT jobject JNICALL Java_com_tidesdb_ColumnFamilyConfig_nativeLoadFromIni(JNIEnv *env, + jclass cls, + jstring iniFile, + jstring sectionName) { const char *ini = (*env)->GetStringUTFChars(env, iniFile, NULL); if (ini == NULL) @@ -1731,7 +1779,7 @@ JNIEXPORT jobject JNICALL Java_com_tidesdb_ColumnFamilyConfig_nativeLoadFromIni( } JNIEXPORT jobject JNICALL Java_com_tidesdb_TidesDBIterator_nativeKeyValue(JNIEnv *env, jclass cls, - jlong handle) + jlong handle) { tidesdb_iter_t *iter = (tidesdb_iter_t *)(uintptr_t)handle; uint8_t *key = NULL; diff --git a/src/test/java/com/tidesdb/TidesDBTest.java b/src/test/java/com/tidesdb/TidesDBTest.java index fdb7d61..5096f05 100644 --- a/src/test/java/com/tidesdb/TidesDBTest.java +++ b/src/test/java/com/tidesdb/TidesDBTest.java @@ -27,6 +27,8 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.CountDownLatch; import static org.junit.jupiter.api.Assertions.*; @@ -2030,4 +2032,318 @@ void testOpenWithS3Config() throws TidesDBException { "exception should explain S3 is unavailable, was: " + ex.getMessage()); } } + + @Test + @Order(57) + void testCommitHookReplaceAndVerify() throws TidesDBException { + Config config = Config.builder(tempDir.resolve("testdb_hook_replace").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + try (TidesDB db = TidesDB.open(config)) { + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("test_cf", cfConfig); + + ColumnFamily cf = db.getColumnFamily("test_cf"); + + List receivedA = new ArrayList<>(); + + cf.setCommitHook((ops, commitSeq) -> { + receivedA.add(ops); + return 0; + }); + + // Commit first put -- hookA should fire + try (Transaction txn = db.beginTransaction()) { + txn.put(cf, "keyA".getBytes(StandardCharsets.UTF_8), + "valueA".getBytes(StandardCharsets.UTF_8)); + txn.commit(); + } + + assertEquals(1, receivedA.size()); + assertEquals(1, receivedA.get(0).length); + assertArrayEquals("keyA".getBytes(StandardCharsets.UTF_8), + receivedA.get(0)[0].getKey()); + assertArrayEquals("valueA".getBytes(StandardCharsets.UTF_8), + receivedA.get(0)[0].getValue()); + + // Replace with a second hook + List receivedB = new ArrayList<>(); + + cf.setCommitHook((ops, commitSeq) -> { + receivedB.add(ops); + return 0; + }); + + // Commit second put -- hookB should fire, hookA should NOT fire again + try (Transaction txn = db.beginTransaction()) { + txn.put(cf, "keyB".getBytes(StandardCharsets.UTF_8), + "valueB".getBytes(StandardCharsets.UTF_8)); + txn.commit(); + } + + assertEquals(1, receivedA.size(), + "Old hook should not fire again after replacement"); + assertEquals(1, receivedB.size()); + assertEquals(1, receivedB.get(0).length); + assertArrayEquals("keyB".getBytes(StandardCharsets.UTF_8), + receivedB.get(0)[0].getKey()); + assertArrayEquals("valueB".getBytes(StandardCharsets.UTF_8), + receivedB.get(0)[0].getValue()); + + cf.clearCommitHook(); + } + } + + @Test + @Order(58) + void testCommitHookOldHookSurvivesFailure() throws TidesDBException { + Config config = Config.builder(tempDir.resolve("testdb_hook_survive").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + try (TidesDB db = TidesDB.open(config)) { + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("test_cf", cfConfig); + + ColumnFamily cf = db.getColumnFamily("test_cf"); + + List receivedA = new ArrayList<>(); + + cf.setCommitHook((ops, commitSeq) -> { + receivedA.add(ops); + return 0; + }); + + // Commit a put -- hookA fires + try (Transaction txn = db.beginTransaction()) { + txn.put(cf, "key1".getBytes(StandardCharsets.UTF_8), + "value1".getBytes(StandardCharsets.UTF_8)); + txn.commit(); + } + assertEquals(1, receivedA.size()); + + // Clear the hook + cf.clearCommitHook(); + + // Commit another put -- hookA should NOT fire + try (Transaction txn = db.beginTransaction()) { + txn.put(cf, "key2".getBytes(StandardCharsets.UTF_8), + "value2".getBytes(StandardCharsets.UTF_8)); + txn.commit(); + } + assertEquals(1, receivedA.size(), + "Hook should not fire after clearing"); + + // Re-register hookA + cf.setCommitHook((ops, commitSeq) -> { + receivedA.add(ops); + return 0; + }); + + // Commit a third put -- re-registered hook should fire + try (Transaction txn = db.beginTransaction()) { + txn.put(cf, "key3".getBytes(StandardCharsets.UTF_8), + "value3".getBytes(StandardCharsets.UTF_8)); + txn.commit(); + } + assertEquals(2, receivedA.size(), + "Re-registered hook should fire"); + assertEquals(1, receivedA.get(1).length); + assertArrayEquals("key3".getBytes(StandardCharsets.UTF_8), + receivedA.get(1)[0].getKey()); + + cf.clearCommitHook(); + } + } + + @Test + @Order(59) + void testCommitHookConcurrentReplaceAndCommit() throws TidesDBException, InterruptedException { + Config config = Config.builder(tempDir.resolve("testdb_hook_concurrent").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + try (TidesDB db = TidesDB.open(config)) { + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("test_cf", cfConfig); + + ColumnFamily cf = db.getColumnFamily("test_cf"); + + AtomicInteger hookCounter = new AtomicInteger(0); + CountDownLatch startLatch = new CountDownLatch(1); + CountDownLatch stopLatch = new CountDownLatch(1); + + // Set an initial hook + cf.setCommitHook((ops, commitSeq) -> { + hookCounter.incrementAndGet(); + return 0; + }); + + // Writer threads that commit puts in a loop + int numWriters = 4; + Thread[] writers = new Thread[numWriters]; + for (int w = 0; w < numWriters; w++) { + final int writerId = w; + writers[w] = new Thread(() -> { + try { + startLatch.await(); + } catch (InterruptedException e) { + return; + } + while (stopLatch.getCount() > 0) { + try { + try (Transaction txn = db.beginTransaction()) { + byte[] key = ("concurrent_key_" + writerId + "_" + + System.nanoTime()).getBytes(StandardCharsets.UTF_8); + txn.put(cf, key, "value".getBytes(StandardCharsets.UTF_8)); + txn.commit(); + } + } catch (TidesDBException e) { + // Expected during hook transitions + } catch (IllegalStateException e) { + // May occur if db is being closed + break; + } + } + }); + writers[w].setDaemon(true); + writers[w].start(); + } + + // Replacer thread that alternates set and clear hook + Thread replacer = new Thread(() -> { + try { + startLatch.await(); + } catch (InterruptedException e) { + return; + } + for (int i = 0; i < 100 && stopLatch.getCount() > 0; i++) { + try { + if (i % 2 == 0) { + cf.setCommitHook((ops, commitSeq) -> { + hookCounter.incrementAndGet(); + return 0; + }); + } else { + cf.clearCommitHook(); + } + } catch (TidesDBException e) { + // Expected during transitions + } catch (IllegalStateException e) { + break; + } + } + }); + replacer.setDaemon(true); + replacer.start(); + + // Start all threads + startLatch.countDown(); + + // Let them run for 3 seconds + Thread.sleep(3000); + + // Signal stop + stopLatch.countDown(); + + // Wait for threads to finish + for (Thread w : writers) { + w.join(5000); + } + replacer.join(5000); + + // No crash occurred -- verify final hook state is coherent + // Do a final commit to verify no crash + try { + cf.setCommitHook((ops, commitSeq) -> { + hookCounter.incrementAndGet(); + return 0; + }); + + try (Transaction txn = db.beginTransaction()) { + txn.put(cf, "final_key".getBytes(StandardCharsets.UTF_8), + "final_value".getBytes(StandardCharsets.UTF_8)); + txn.commit(); + } + + assertTrue(hookCounter.get() > 0, + "At least one hook invocation should have occurred"); + + cf.clearCommitHook(); + } catch (TidesDBException e) { + fail("Final commit after concurrent stress should not throw: " + e.getMessage()); + } + } + } + + @Test + @Order(60) + void testCommitHookRepeatedTransitions() throws TidesDBException { + Config config = Config.builder(tempDir.resolve("testdb_hook_transitions").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + try (TidesDB db = TidesDB.open(config)) { + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("test_cf", cfConfig); + + ColumnFamily cf = db.getColumnFamily("test_cf"); + + final int iterations = 50; + + for (int i = 0; i < iterations; i++) { + List received = new ArrayList<>(); + + cf.setCommitHook((ops, commitSeq) -> { + received.add(ops); + return 0; + }); + + // Commit with hook active -- hook should fire + try (Transaction txn = db.beginTransaction()) { + byte[] key = ("key_set_" + i).getBytes(StandardCharsets.UTF_8); + byte[] value = ("value_set_" + i).getBytes(StandardCharsets.UTF_8); + txn.put(cf, key, value); + txn.commit(); + } + + assertEquals(1, received.size(), + "Hook should fire at iteration " + i); + assertArrayEquals(("key_set_" + i).getBytes(StandardCharsets.UTF_8), + received.get(0)[0].getKey()); + + // Clear the hook + cf.clearCommitHook(); + + // Commit with hook cleared -- hook should NOT fire + received.clear(); + try (Transaction txn = db.beginTransaction()) { + byte[] key = ("key_clear_" + i).getBytes(StandardCharsets.UTF_8); + byte[] value = ("value_clear_" + i).getBytes(StandardCharsets.UTF_8); + txn.put(cf, key, value); + txn.commit(); + } + + assertEquals(0, received.size(), + "Hook should not fire after clearing at iteration " + i); + } + } + } } From e634b01d9583a64144707623f32ea9b4a55c8d29 Mon Sep 17 00:00:00 2001 From: dhoard Date: Sun, 12 Jul 2026 22:07:58 -0400 Subject: [PATCH 2/7] fix: Fixed commit hook shutdown/handle leak Signed-off-by: dhoard --- src/main/java/com/tidesdb/ColumnFamily.java | 49 ++++ src/main/java/com/tidesdb/TidesDB.java | 51 +++- src/test/java/com/tidesdb/TidesDBTest.java | 271 ++++++++++++++++++++ 3 files changed, 369 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/tidesdb/ColumnFamily.java b/src/main/java/com/tidesdb/ColumnFamily.java index e07a57f..30ce8c6 100644 --- a/src/main/java/com/tidesdb/ColumnFamily.java +++ b/src/main/java/com/tidesdb/ColumnFamily.java @@ -31,10 +31,25 @@ public class ColumnFamily { private final long nativeHandle; private final String name; private long commitHookCtxHandle = 0; + private final TidesDB owner; ColumnFamily(long nativeHandle, String name) { + this(nativeHandle, name, null); + } + + ColumnFamily(long nativeHandle, String name, TidesDB owner) { this.nativeHandle = nativeHandle; this.name = name; + this.owner = owner; + } + + /** + * Checks that the owning database is open. Throws if the owner is closed. + */ + void checkOwnerOpen() { + if (owner != null && owner.isClosed()) { + throw new IllegalStateException("TidesDB instance is closed"); + } } /** @@ -53,6 +68,7 @@ public String getName() { * @throws TidesDBException if the stats cannot be retrieved */ public Stats getStats() throws TidesDBException { + checkOwnerOpen(); return nativeGetStats(nativeHandle); } @@ -62,6 +78,7 @@ public Stats getStats() throws TidesDBException { * @throws TidesDBException if compaction fails */ public void compact() throws TidesDBException { + checkOwnerOpen(); nativeCompact(nativeHandle); } @@ -80,6 +97,7 @@ public void compact() throws TidesDBException { * or the merge fails */ public void compactRange(byte[] startKey, byte[] endKey) throws TidesDBException { + checkOwnerOpen(); nativeCompactRange(nativeHandle, startKey, endKey); } @@ -89,6 +107,7 @@ public void compactRange(byte[] startKey, byte[] endKey) throws TidesDBException * @throws TidesDBException if flush fails */ public void flushMemtable() throws TidesDBException { + checkOwnerOpen(); nativeFlushMemtable(nativeHandle); } @@ -98,6 +117,7 @@ public void flushMemtable() throws TidesDBException { * @return true if flushing is in progress */ public boolean isFlushing() { + checkOwnerOpen(); return nativeIsFlushing(nativeHandle); } @@ -107,6 +127,7 @@ public boolean isFlushing() { * @return true if compaction is in progress */ public boolean isCompacting() { + checkOwnerOpen(); return nativeIsCompacting(nativeHandle); } @@ -130,6 +151,7 @@ public boolean isCompacting() { * @throws TidesDBException if the update fails */ public void updateRuntimeConfig(ColumnFamilyConfig config, boolean persistToDisk) throws TidesDBException { + checkOwnerOpen(); if (config == null) { throw new IllegalArgumentException("Config cannot be null"); } @@ -152,6 +174,7 @@ public void updateRuntimeConfig(ColumnFamilyConfig config, boolean persistToDisk * @throws TidesDBException if the purge fails */ public void purge() throws TidesDBException { + checkOwnerOpen(); nativePurge(nativeHandle); } @@ -162,6 +185,7 @@ public void purge() throws TidesDBException { * @throws TidesDBException if the WAL sync fails */ public void syncWal() throws TidesDBException { + checkOwnerOpen(); nativeSyncWal(nativeHandle); } @@ -177,6 +201,7 @@ public void syncWal() throws TidesDBException { * @throws TidesDBException if the estimation fails */ public double rangeCost(byte[] keyA, byte[] keyB) throws TidesDBException { + checkOwnerOpen(); if (keyA == null || keyA.length == 0) { throw new IllegalArgumentException("keyA cannot be null or empty"); } @@ -199,10 +224,15 @@ public double rangeCost(byte[] keyA, byte[] keyB) throws TidesDBException { * @throws TidesDBException if the hook cannot be set */ public void setCommitHook(CommitHook hook) throws TidesDBException { + checkOwnerOpen(); if (hook == null) { throw new IllegalArgumentException("Hook cannot be null, use clearCommitHook() instead"); } + boolean firstInstall = (commitHookCtxHandle == 0); commitHookCtxHandle = nativeSetCommitHook(nativeHandle, hook, commitHookCtxHandle); + if (firstInstall && owner != null) { + owner.registerHookColumnFamily(this); + } } /** @@ -212,7 +242,26 @@ public void setCommitHook(CommitHook hook) throws TidesDBException { * @throws TidesDBException if the hook cannot be cleared */ public void clearCommitHook() throws TidesDBException { + checkOwnerOpen(); commitHookCtxHandle = nativeSetCommitHook(nativeHandle, null, commitHookCtxHandle); + if (owner != null) { + owner.unregisterHookColumnFamily(this); + } + } + + /** + * Best-effort hook detach during database close. The hook is removed from + * the engine without caller interaction. Errors are swallowed. + */ + void clearHookOnClose() { + if (commitHookCtxHandle != 0) { + try { + nativeSetCommitHook(nativeHandle, null, commitHookCtxHandle); + } catch (TidesDBException ignored) { + // Best-effort during shutdown. + } + commitHookCtxHandle = 0; + } } long getNativeHandle() { diff --git a/src/main/java/com/tidesdb/TidesDB.java b/src/main/java/com/tidesdb/TidesDB.java index c5a10c7..a758969 100644 --- a/src/main/java/com/tidesdb/TidesDB.java +++ b/src/main/java/com/tidesdb/TidesDB.java @@ -19,6 +19,10 @@ package com.tidesdb; import java.io.Closeable; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; /** * TidesDB is the main database class providing access to TidesDB functionality. @@ -32,6 +36,7 @@ public class TidesDB implements Closeable { private long nativeHandle; private boolean closed = false; + private final Set hookBearingCFs = new HashSet<>(); private TidesDB(long nativeHandle) { this.nativeHandle = nativeHandle; @@ -134,9 +139,26 @@ public static boolean isS3Available() { @Override public void close() { if (!closed && nativeHandle != 0) { + closed = true; + + // Drain all registered hooks. Loop because a concurrent setCommitHook + // may have passed checkOwnerOpen before closed=true but hasn't + // registered yet; its registration is rejected (closed is true), but a + // retry loop guarantees the set is empty before calling nativeClose. + while (true) { + List batch; + synchronized (hookBearingCFs) { + if (hookBearingCFs.isEmpty()) break; + batch = new ArrayList<>(hookBearingCFs); + hookBearingCFs.clear(); + } + for (ColumnFamily cf : batch) { + cf.clearHookOnClose(); + } + } + nativeClose(nativeHandle); nativeHandle = 0; - closed = true; } } @@ -212,7 +234,7 @@ public ColumnFamily getColumnFamily(String name) throws TidesDBException { throw new IllegalArgumentException("Column family name cannot be null or empty"); } long cfHandle = nativeGetColumnFamily(nativeHandle, name); - return new ColumnFamily(cfHandle, name); + return new ColumnFamily(cfHandle, name, this); } /** @@ -436,6 +458,31 @@ private void checkNotClosed() { } } + /** + * Reports whether this database instance has been closed. + */ + boolean isClosed() { + return closed; + } + + /** + * Registers a column family that has an installed commit hook. + */ + void registerHookColumnFamily(ColumnFamily cf) { + synchronized (hookBearingCFs) { + if (!closed) hookBearingCFs.add(cf); + } + } + + /** + * Unregisters a column family whose commit hook has been cleared. + */ + void unregisterHookColumnFamily(ColumnFamily cf) { + synchronized (hookBearingCFs) { + hookBearingCFs.remove(cf); + } + } + long getNativeHandle() { return nativeHandle; } diff --git a/src/test/java/com/tidesdb/TidesDBTest.java b/src/test/java/com/tidesdb/TidesDBTest.java index 5096f05..606a54b 100644 --- a/src/test/java/com/tidesdb/TidesDBTest.java +++ b/src/test/java/com/tidesdb/TidesDBTest.java @@ -29,6 +29,7 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import static org.junit.jupiter.api.Assertions.*; @@ -2346,4 +2347,274 @@ void testCommitHookRepeatedTransitions() throws TidesDBException { } } } + + @Test + @Order(61) + void testCloseWithInstalledHookReleasesCallback() throws TidesDBException { + Config config = Config.builder(tempDir.resolve("testdb_close_hook_leak").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + List received = new ArrayList<>(); + + try (TidesDB db = TidesDB.open(config)) { + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("test_cf", cfConfig); + + ColumnFamily cf = db.getColumnFamily("test_cf"); + + cf.setCommitHook((ops, commitSeq) -> { + received.add(ops); + return 0; + }); + + // Commit some data so the hook is exercised + try (Transaction txn = db.beginTransaction()) { + txn.put(cf, "key1".getBytes(StandardCharsets.UTF_8), + "value1".getBytes(StandardCharsets.UTF_8)); + txn.commit(); + } + + assertEquals(1, received.size()); + + // close() via try-with-resources WITHOUT calling clearCommitHook() + } + + // After close, trigger GC and verify no callbacks fire post-close + System.gc(); + + // The hook should not have fired again after close + assertEquals(1, received.size(), + "Hook callback should not fire after database close"); + } + + @Test + @Order(62) + void testCloseWithInstalledHookIdempotent() throws TidesDBException { + Config config = Config.builder(tempDir.resolve("testdb_close_idempotent").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + TidesDB db = TidesDB.open(config); + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("test_cf", cfConfig); + + ColumnFamily cf = db.getColumnFamily("test_cf"); + + cf.setCommitHook((ops, commitSeq) -> { + return 0; + }); + + // First close + db.close(); + + // Second close should not throw + assertDoesNotThrow(db::close); + } + + @Test + @Order(63) + void testColumnFamilyOperationsThrowAfterOwnerClose() throws TidesDBException { + Config config = Config.builder(tempDir.resolve("testdb_cf_after_close").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + ColumnFamily cf; + try (TidesDB db = TidesDB.open(config)) { + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("test_cf", cfConfig); + cf = db.getColumnFamily("test_cf"); + } + // db is now closed; cf still references it + + assertThrows(IllegalStateException.class, cf::getStats); + assertThrows(IllegalStateException.class, cf::compact); + assertThrows(IllegalStateException.class, + () -> cf.compactRange("a".getBytes(), "z".getBytes())); + assertThrows(IllegalStateException.class, cf::flushMemtable); + assertThrows(IllegalStateException.class, cf::isFlushing); + assertThrows(IllegalStateException.class, cf::isCompacting); + assertThrows(IllegalStateException.class, + () -> cf.updateRuntimeConfig(ColumnFamilyConfig.defaultConfig(), false)); + assertThrows(IllegalStateException.class, + () -> cf.rangeCost("a".getBytes(), "z".getBytes())); + assertThrows(IllegalStateException.class, + () -> cf.setCommitHook((ops, seq) -> 0)); + assertThrows(IllegalStateException.class, cf::clearCommitHook); + assertThrows(IllegalStateException.class, cf::purge); + assertThrows(IllegalStateException.class, cf::syncWal); + } + + @Test + @Order(64) + void testSetClearHookAfterCloseThrows() throws TidesDBException { + Config config = Config.builder(tempDir.resolve("testdb_hook_after_close").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + ColumnFamily cf; + try (TidesDB db = TidesDB.open(config)) { + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("test_cf", cfConfig); + cf = db.getColumnFamily("test_cf"); + } + // db is now closed + + assertThrows(IllegalStateException.class, + () -> cf.setCommitHook((ops, seq) -> 0)); + assertThrows(IllegalStateException.class, cf::clearCommitHook); + } + + @Test + @Order(65) + void testMultipleColumnFamiliesWithHooksCloseCleanly() throws TidesDBException { + Config config = Config.builder(tempDir.resolve("testdb_multi_cf_hooks").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + List hook1Received = new ArrayList<>(); + List hook2Received = new ArrayList<>(); + + try (TidesDB db = TidesDB.open(config)) { + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("cf1", cfConfig); + db.createColumnFamily("cf2", cfConfig); + db.createColumnFamily("cf3", cfConfig); + + ColumnFamily cf1 = db.getColumnFamily("cf1"); + ColumnFamily cf2 = db.getColumnFamily("cf2"); + ColumnFamily cf3 = db.getColumnFamily("cf3"); + + // Install hooks on cf1 and cf2, leave cf3 without a hook + cf1.setCommitHook((ops, seq) -> { + hook1Received.add(ops); + return 0; + }); + cf2.setCommitHook((ops, seq) -> { + hook2Received.add(ops); + return 0; + }); + + // Commit data to all three CFs + try (Transaction txn = db.beginTransaction()) { + txn.put(cf1, "k1".getBytes(), "v1".getBytes()); + txn.put(cf2, "k2".getBytes(), "v2".getBytes()); + txn.put(cf3, "k3".getBytes(), "v3".getBytes()); + txn.commit(); + } + + assertEquals(1, hook1Received.size()); + assertEquals(1, hook2Received.size()); + + // close() via try-with-resources WITHOUT clearing hooks + } + + // After close, no deferred callbacks should fire + System.gc(); + assertEquals(1, hook1Received.size(), + "Hook1 should not fire after close"); + assertEquals(1, hook2Received.size(), + "Hook2 should not fire after close"); + } + + @Test + @Order(66) + void testSetClearHookNotRacingClose() throws TidesDBException, InterruptedException { + Config config = Config.builder(tempDir.resolve("testdb_hook_race_close").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + try (TidesDB db = TidesDB.open(config)) { + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("test_cf", cfConfig); + + ColumnFamily cf = db.getColumnFamily("test_cf"); + + // Set initial hook + cf.setCommitHook((ops, seq) -> { + return 0; + }); + + CountDownLatch startLatch = new CountDownLatch(1); + CountDownLatch stopLatch = new CountDownLatch(1); + + // Writer thread that commits in a loop + Thread writer = new Thread(() -> { + try { + startLatch.await(); + } catch (InterruptedException e) { + return; + } + while (stopLatch.getCount() > 0) { + try (Transaction txn = db.beginTransaction()) { + byte[] key = ("key_" + System.nanoTime()).getBytes(StandardCharsets.UTF_8); + txn.put(cf, key, "value".getBytes(StandardCharsets.UTF_8)); + txn.commit(); + } catch (TidesDBException | IllegalStateException e) { + // Expected during close + } + } + }); + writer.setDaemon(true); + + // Replacer thread that alternates set/clear hook + Thread replacer = new Thread(() -> { + try { + startLatch.await(); + } catch (InterruptedException e) { + return; + } + for (int i = 0; i < 200 && stopLatch.getCount() > 0; i++) { + try { + if (i % 2 == 0) { + cf.setCommitHook((ops, seq) -> 0); + } else { + cf.clearCommitHook(); + } + } catch (TidesDBException | IllegalStateException e) { + // Expected during close + } + } + }); + replacer.setDaemon(true); + + writer.start(); + replacer.start(); + startLatch.countDown(); + + // Let them run for ~500ms then close + Thread.sleep(500); + + // Close should not hang, crash, or throw + assertDoesNotThrow(db::close); + + stopLatch.countDown(); + writer.join(5000); + replacer.join(5000); + } + } } From 223ed6e3fcd1c0e1dba6d130594513988d0355ea Mon Sep 17 00:00:00 2001 From: dhoard Date: Sun, 12 Jul 2026 22:32:05 -0400 Subject: [PATCH 3/7] fix: Fixed connector leaks when open fails Signed-off-by: dhoard --- src/main/c/com_tidesdb_TidesDB.c | 36 ++++++++++++++++++++++ src/test/java/com/tidesdb/TidesDBTest.java | 29 +++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/src/main/c/com_tidesdb_TidesDB.c b/src/main/c/com_tidesdb_TidesDB.c index 122c455..b8f982c 100644 --- a/src/main/c/com_tidesdb_TidesDB.c +++ b/src/main/c/com_tidesdb_TidesDB.c @@ -25,6 +25,26 @@ #include #endif +/* ABI-compatible overlay for tidesdb_objstore_t. + * The FFI header (db.h) defines this type as opaque. The real layout is published + * in tidesdb/objstore.h and is part of the stable library ABI. This overlay lets + * us call destroy(ctx) + free() on the connector failure path without pulling in + * the full objstore.h header (which conflicts with db.h). */ +struct _tidesdb_objstore_overlay +{ + int backend; + void *put; + void *get; + void *range_get; + void *delete_object; + void *exists; + void *list; + void *put_if; + void *head; + void (*destroy)(void *ctx); + void *ctx; +}; + static void throwTidesDBException(JNIEnv *env, int errorCode, const char *message) { jclass exClass = (*env)->FindClass(env, "com/tidesdb/TidesDBException"); @@ -181,6 +201,22 @@ JNIEXPORT jlong JNICALL Java_com_tidesdb_TidesDB_nativeOpen( if (result != TDB_SUCCESS) { + /* Destroy any connector that was not transferred to tidesdb_open. + * Externally-provided connectors (e.g. S3) and locally-created + * filesystem connectors are both owned by this JNI call until + * tidesdb_open succeeds. On failure we must release them. */ + if (obj_store != NULL) + { + struct _tidesdb_objstore_overlay *overlay = + (struct _tidesdb_objstore_overlay *)obj_store; + if (overlay->destroy) + { + overlay->destroy(overlay->ctx); + } + free(obj_store); + obj_store = NULL; + } + throwTidesDBException(env, result, getErrorMessage(result)); return 0; } diff --git a/src/test/java/com/tidesdb/TidesDBTest.java b/src/test/java/com/tidesdb/TidesDBTest.java index 606a54b..d4b7787 100644 --- a/src/test/java/com/tidesdb/TidesDBTest.java +++ b/src/test/java/com/tidesdb/TidesDBTest.java @@ -2537,6 +2537,35 @@ void testMultipleColumnFamiliesWithHooksCloseCleanly() throws TidesDBException { "Hook2 should not fire after close"); } + @Test + @Order(69) + void testRepeatedS3OpenFailureDoesNotCrash() { + if (!TidesDB.isS3Available()) { + // S3 support not compiled in; nothing to verify. + return; + } + + // Use a port that is almost certainly unused (distinct from 9000 used by other tests) + S3Config s3 = S3Config.builder() + .endpoint("127.0.0.1:19000") + .bucket("tidesdb-test") + .accessKey("minioadmin") + .secretKey("minioadmin") + .usePathStyle(true) + .useSsl(false) + .build(); + + Config config = Config.builder(tempDir.resolve("testdb_s3_leak").toString()) + .objectStoreS3Config(s3) + .build(); + + // Each iteration creates an S3 connector then fails tidesdb_open. + // Without the fix the connector leaks native memory. + for (int i = 0; i < 50; i++) { + assertThrows(TidesDBException.class, () -> TidesDB.open(config)); + } + } + @Test @Order(66) void testSetClearHookNotRacingClose() throws TidesDBException, InterruptedException { From bac9862e888814f4a568410522b7bf1417817352 Mon Sep 17 00:00:00 2001 From: dhoard Date: Sun, 12 Jul 2026 22:58:33 -0400 Subject: [PATCH 4/7] fix: Fixed unchecked JNI buffer acquisition Signed-off-by: dhoard --- src/main/c/com_tidesdb_TidesDB.c | 380 ++++++++++++++++----- src/test/java/com/tidesdb/TidesDBTest.java | 136 ++++++++ 2 files changed, 431 insertions(+), 85 deletions(-) diff --git a/src/main/c/com_tidesdb_TidesDB.c b/src/main/c/com_tidesdb_TidesDB.c index b8f982c..87af5a0 100644 --- a/src/main/c/com_tidesdb_TidesDB.c +++ b/src/main/c/com_tidesdb_TidesDB.c @@ -25,6 +25,13 @@ #include #endif +#ifndef JSIZE_MAX +#define JSIZE_MAX (((jsize)~0) >> 1) +#endif + +/* Forward declaration */ +static jobject buildCfConfigObject(JNIEnv *env, const tidesdb_column_family_config_t *cfg); + /* ABI-compatible overlay for tidesdb_objstore_t. * The FFI header (db.h) defines this type as opaque. The real layout is published * in tidesdb/objstore.h and is part of the stable library ABI. This overlay lets @@ -66,6 +73,11 @@ static void throwTidesDBException(JNIEnv *env, int errorCode, const char *messag (*env)->Throw(env, exception); } +static int jvm_exception_pending(JNIEnv *env) +{ + return (*env)->ExceptionCheck(env) == JNI_TRUE; +} + static const char *getErrorMessage(int code) { switch (code) @@ -134,6 +146,14 @@ JNIEXPORT jlong JNICALL Java_com_tidesdb_TidesDB_nativeOpen( else if (objectStoreFsPath != NULL) { fs_path = (*env)->GetStringUTFChars(env, objectStoreFsPath, NULL); + if (objectStoreFsPath != NULL && fs_path == NULL) + { + (*env)->ReleaseStringUTFChars(env, dbPath, path); + if (!jvm_exception_pending(env)) + throwTidesDBException(env, TDB_ERR_MEMORY, + "Failed to get object store filesystem path"); + return 0; + } if (fs_path != NULL) { obj_store = tidesdb_objstore_fs_create(fs_path); @@ -145,6 +165,14 @@ JNIEXPORT jlong JNICALL Java_com_tidesdb_TidesDB_nativeOpen( if (oscLocalCachePath != NULL) { cache_path = (*env)->GetStringUTFChars(env, oscLocalCachePath, NULL); + if (oscLocalCachePath != NULL && cache_path == NULL) + { + if (fs_path != NULL) (*env)->ReleaseStringUTFChars(env, objectStoreFsPath, fs_path); + (*env)->ReleaseStringUTFChars(env, dbPath, path); + if (!jvm_exception_pending(env)) + throwTidesDBException(env, TDB_ERR_MEMORY, "Failed to get local cache path"); + return 0; + } } tidesdb_objstore_config_t os_cfg = { @@ -262,15 +290,67 @@ JNIEXPORT jlong JNICALL Java_com_tidesdb_TidesDB_nativeObjstoreS3Create( return 0; } - /* required strings */ + /* required strings with NULL checks */ const char *c_endpoint = endpoint ? (*env)->GetStringUTFChars(env, endpoint, NULL) : NULL; + if (endpoint != NULL && c_endpoint == NULL) return 0; + const char *c_bucket = bucket ? (*env)->GetStringUTFChars(env, bucket, NULL) : NULL; + if (bucket != NULL && c_bucket == NULL) + { + if (c_endpoint) (*env)->ReleaseStringUTFChars(env, endpoint, c_endpoint); + return 0; + } + const char *c_access = accessKey ? (*env)->GetStringUTFChars(env, accessKey, NULL) : NULL; + if (accessKey != NULL && c_access == NULL) + { + if (c_endpoint) (*env)->ReleaseStringUTFChars(env, endpoint, c_endpoint); + if (c_bucket) (*env)->ReleaseStringUTFChars(env, bucket, c_bucket); + return 0; + } + const char *c_secret = secretKey ? (*env)->GetStringUTFChars(env, secretKey, NULL) : NULL; + if (secretKey != NULL && c_secret == NULL) + { + if (c_endpoint) (*env)->ReleaseStringUTFChars(env, endpoint, c_endpoint); + if (c_bucket) (*env)->ReleaseStringUTFChars(env, bucket, c_bucket); + if (c_access) (*env)->ReleaseStringUTFChars(env, accessKey, c_access); + return 0; + } + /* optional strings */ const char *c_prefix = prefix ? (*env)->GetStringUTFChars(env, prefix, NULL) : NULL; + if (prefix != NULL && c_prefix == NULL) + { + if (c_endpoint) (*env)->ReleaseStringUTFChars(env, endpoint, c_endpoint); + if (c_bucket) (*env)->ReleaseStringUTFChars(env, bucket, c_bucket); + if (c_access) (*env)->ReleaseStringUTFChars(env, accessKey, c_access); + if (c_secret) (*env)->ReleaseStringUTFChars(env, secretKey, c_secret); + return 0; + } + const char *c_region = region ? (*env)->GetStringUTFChars(env, region, NULL) : NULL; + if (region != NULL && c_region == NULL) + { + if (c_endpoint) (*env)->ReleaseStringUTFChars(env, endpoint, c_endpoint); + if (c_bucket) (*env)->ReleaseStringUTFChars(env, bucket, c_bucket); + if (c_access) (*env)->ReleaseStringUTFChars(env, accessKey, c_access); + if (c_secret) (*env)->ReleaseStringUTFChars(env, secretKey, c_secret); + if (c_prefix) (*env)->ReleaseStringUTFChars(env, prefix, c_prefix); + return 0; + } + const char *c_ca = tlsCaPath ? (*env)->GetStringUTFChars(env, tlsCaPath, NULL) : NULL; + if (tlsCaPath != NULL && c_ca == NULL) + { + if (c_endpoint) (*env)->ReleaseStringUTFChars(env, endpoint, c_endpoint); + if (c_bucket) (*env)->ReleaseStringUTFChars(env, bucket, c_bucket); + if (c_access) (*env)->ReleaseStringUTFChars(env, accessKey, c_access); + if (c_secret) (*env)->ReleaseStringUTFChars(env, secretKey, c_secret); + if (c_prefix) (*env)->ReleaseStringUTFChars(env, prefix, c_prefix); + if (c_region) (*env)->ReleaseStringUTFChars(env, region, c_region); + return 0; + } tidesdb_objstore_s3_config_t cfg = {.endpoint = c_endpoint, .bucket = c_bucket, @@ -450,12 +530,33 @@ JNIEXPORT jobjectArray JNICALL Java_com_tidesdb_TidesDB_nativeListColumnFamilies } jclass stringClass = (*env)->FindClass(env, "java/lang/String"); + if (stringClass == NULL) + { + for (int i = 0; i < count; i++) free(names[i]); + free(names); + return NULL; + } + jobjectArray array = (*env)->NewObjectArray(env, count, stringClass, NULL); + if (array == NULL) + { + for (int i = 0; i < count; i++) free(names[i]); + free(names); + return NULL; + } for (int i = 0; i < count; i++) { jstring str = (*env)->NewStringUTF(env, names[i]); + if (str == NULL) + { + /* OOM: pending JVM exception. Free remaining names and return. */ + for (int j = i; j < count; j++) free(names[j]); + free(names); + return NULL; + } (*env)->SetObjectArrayElement(env, array, i, str); + (*env)->DeleteLocalRef(env, str); free(names[i]); } free(names); @@ -510,7 +611,10 @@ JNIEXPORT jobject JNICALL Java_com_tidesdb_TidesDB_nativeGetCacheStats(JNIEnv *e } jclass cacheStatsClass = (*env)->FindClass(env, "com/tidesdb/CacheStats"); + if (cacheStatsClass == NULL) return NULL; + jmethodID constructor = (*env)->GetMethodID(env, cacheStatsClass, "", "(ZJJJJDJ)V"); + if (constructor == NULL) return NULL; return (*env)->NewObject(env, cacheStatsClass, constructor, stats.enabled != 0, (jlong)stats.total_entries, (jlong)stats.total_bytes, @@ -670,92 +774,133 @@ JNIEXPORT jobject JNICALL Java_com_tidesdb_ColumnFamily_nativeGetStats(JNIEnv *e } jlongArray levelSizes = (*env)->NewLongArray(env, stats->num_levels); + if (levelSizes == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } + + jlong *sizes = NULL; if (stats->level_sizes != NULL) { - jlong *sizes = malloc(stats->num_levels * sizeof(jlong)); + sizes = malloc(stats->num_levels * sizeof(jlong)); + if (sizes == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } for (int i = 0; i < stats->num_levels; i++) { sizes[i] = (jlong)stats->level_sizes[i]; } (*env)->SetLongArrayRegion(env, levelSizes, 0, stats->num_levels, sizes); free(sizes); + sizes = NULL; } jintArray levelNumSSTables = (*env)->NewIntArray(env, stats->num_levels); + if (levelNumSSTables == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } + + jint *nums = NULL; if (stats->level_num_sstables != NULL) { - jint *nums = malloc(stats->num_levels * sizeof(jint)); + nums = malloc(stats->num_levels * sizeof(jint)); + if (nums == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } for (int i = 0; i < stats->num_levels; i++) { nums[i] = stats->level_num_sstables[i]; } (*env)->SetIntArrayRegion(env, levelNumSSTables, 0, stats->num_levels, nums); free(nums); + nums = NULL; } jlongArray levelKeyCounts = (*env)->NewLongArray(env, stats->num_levels); + if (levelKeyCounts == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } + + jlong *counts = NULL; if (stats->level_key_counts != NULL) { - jlong *counts = malloc(stats->num_levels * sizeof(jlong)); + counts = malloc(stats->num_levels * sizeof(jlong)); + if (counts == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } for (int i = 0; i < stats->num_levels; i++) { counts[i] = (jlong)stats->level_key_counts[i]; } (*env)->SetLongArrayRegion(env, levelKeyCounts, 0, stats->num_levels, counts); free(counts); + counts = NULL; } jlongArray levelTombstoneCounts = (*env)->NewLongArray(env, stats->num_levels); + if (levelTombstoneCounts == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } + + jlong *tombstoneCounts = NULL; if (stats->level_tombstone_counts != NULL) { - jlong *counts = malloc(stats->num_levels * sizeof(jlong)); + tombstoneCounts = malloc(stats->num_levels * sizeof(jlong)); + if (tombstoneCounts == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } for (int i = 0; i < stats->num_levels; i++) { - counts[i] = (jlong)stats->level_tombstone_counts[i]; + tombstoneCounts[i] = (jlong)stats->level_tombstone_counts[i]; } - (*env)->SetLongArrayRegion(env, levelTombstoneCounts, 0, stats->num_levels, counts); - free(counts); + (*env)->SetLongArrayRegion(env, levelTombstoneCounts, 0, stats->num_levels, + tombstoneCounts); + free(tombstoneCounts); + tombstoneCounts = NULL; } /* Build ColumnFamilyConfig from stats->config so callers can round-trip CF settings */ jobject cfConfigObj = NULL; if (stats->config != NULL) { - jclass cfConfigClass = (*env)->FindClass(env, "com/tidesdb/ColumnFamilyConfig"); - jmethodID fromNative = (*env)->GetStaticMethodID( - env, cfConfigClass, "fromNative", - "(JJIIJIZDZIIIJLjava/lang/String;IFIJIIDJZZZ)Lcom/tidesdb/ColumnFamilyConfig;"); - - jstring comparatorName = (*env)->NewStringUTF(env, stats->config->comparator_name); - - cfConfigObj = (*env)->CallStaticObjectMethod( - env, cfConfigClass, fromNative, (jlong)stats->config->write_buffer_size, - (jlong)stats->config->level_size_ratio, (jint)stats->config->min_levels, - (jint)stats->config->dividing_level_offset, (jlong)stats->config->klog_value_threshold, - (jint)stats->config->compression_algorithm, - stats->config->enable_bloom_filter != 0 ? JNI_TRUE : JNI_FALSE, - (jdouble)stats->config->bloom_fpr, - stats->config->enable_block_indexes != 0 ? JNI_TRUE : JNI_FALSE, - (jint)stats->config->index_sample_ratio, (jint)stats->config->block_index_prefix_len, - (jint)stats->config->sync_mode, (jlong)stats->config->sync_interval_us, comparatorName, - (jint)stats->config->skip_list_max_level, (jfloat)stats->config->skip_list_probability, - (jint)stats->config->default_isolation_level, (jlong)stats->config->min_disk_space, - (jint)stats->config->l1_file_count_trigger, - (jint)stats->config->l0_queue_stall_threshold, - (jdouble)stats->config->tombstone_density_trigger, - (jlong)stats->config->tombstone_density_min_entries, - stats->config->use_btree != 0 ? JNI_TRUE : JNI_FALSE, - stats->config->object_lazy_compaction != 0 ? JNI_TRUE : JNI_FALSE, - stats->config->object_prefetch_compaction != 0 ? JNI_TRUE : JNI_FALSE); - - (*env)->DeleteLocalRef(env, comparatorName); - (*env)->DeleteLocalRef(env, cfConfigClass); + cfConfigObj = buildCfConfigObject(env, stats->config); + if (cfConfigObj == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } } jclass statsClass = (*env)->FindClass(env, "com/tidesdb/Stats"); + if (statsClass == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } + jmethodID constructor = (*env)->GetMethodID(env, statsClass, "", "(IJ[J[ILcom/tidesdb/ColumnFamilyConfig;JJDD[JDDZJIDJD[JDIJJJJJJJ)V"); + if (constructor == NULL) + { + tidesdb_free_stats(stats); + return NULL; + } jobject statsObj = (*env)->NewObject( env, statsClass, constructor, stats->num_levels, (jlong)stats->memtable_size, levelSizes, @@ -803,6 +948,7 @@ JNIEXPORT void JNICALL Java_com_tidesdb_ColumnFamily_nativeCompactRange(JNIEnv * if (startLen > 0) { startBytes = (*env)->GetByteArrayElements(env, startKey, NULL); + if (startBytes == NULL) return; /* JVM exception already pending */ } } @@ -814,25 +960,23 @@ JNIEXPORT void JNICALL Java_com_tidesdb_ColumnFamily_nativeCompactRange(JNIEnv * if (endLen > 0) { endBytes = (*env)->GetByteArrayElements(env, endKey, NULL); + if (endBytes == NULL) + { + if (startBytes != NULL) + (*env)->ReleaseByteArrayElements(env, startKey, startBytes, JNI_ABORT); + return; /* JVM exception already pending */ + } } } int result = tidesdb_compact_range(cf, (const uint8_t *)startBytes, (size_t)startLen, (const uint8_t *)endBytes, (size_t)endLen); - if (startBytes != NULL) - { - (*env)->ReleaseByteArrayElements(env, startKey, startBytes, JNI_ABORT); - } - if (endBytes != NULL) - { - (*env)->ReleaseByteArrayElements(env, endKey, endBytes, JNI_ABORT); - } + if (startBytes != NULL) (*env)->ReleaseByteArrayElements(env, startKey, startBytes, JNI_ABORT); + if (endBytes != NULL) (*env)->ReleaseByteArrayElements(env, endKey, endBytes, JNI_ABORT); - if (result != TDB_SUCCESS) - { + if (result != TDB_SUCCESS && !jvm_exception_pending(env)) throwTidesDBException(env, result, getErrorMessage(result)); - } } JNIEXPORT void JNICALL Java_com_tidesdb_ColumnFamily_nativeFlushMemtable(JNIEnv *env, jclass cls, @@ -895,7 +1039,14 @@ JNIEXPORT void JNICALL Java_com_tidesdb_Transaction_nativePut(JNIEnv *env, jclas jsize valueLen = (*env)->GetArrayLength(env, value); jbyte *keyBytes = (*env)->GetByteArrayElements(env, key, NULL); + if (keyBytes == NULL) return; /* JVM exception already pending */ + jbyte *valueBytes = (*env)->GetByteArrayElements(env, value, NULL); + if (valueBytes == NULL) + { + (*env)->ReleaseByteArrayElements(env, key, keyBytes, JNI_ABORT); + return; /* JVM exception already pending */ + } int result = tidesdb_txn_put(txn, cf, (uint8_t *)keyBytes, keyLen, (uint8_t *)valueBytes, valueLen, (time_t)ttl); @@ -903,10 +1054,8 @@ JNIEXPORT void JNICALL Java_com_tidesdb_Transaction_nativePut(JNIEnv *env, jclas (*env)->ReleaseByteArrayElements(env, key, keyBytes, JNI_ABORT); (*env)->ReleaseByteArrayElements(env, value, valueBytes, JNI_ABORT); - if (result != TDB_SUCCESS) - { + if (result != TDB_SUCCESS && !jvm_exception_pending(env)) throwTidesDBException(env, result, getErrorMessage(result)); - } } JNIEXPORT jbyteArray JNICALL Java_com_tidesdb_Transaction_nativeGet(JNIEnv *env, jclass cls, @@ -918,6 +1067,7 @@ JNIEXPORT jbyteArray JNICALL Java_com_tidesdb_Transaction_nativeGet(JNIEnv *env, jsize keyLen = (*env)->GetArrayLength(env, key); jbyte *keyBytes = (*env)->GetByteArrayElements(env, key, NULL); + if (keyBytes == NULL) return NULL; /* JVM exception already pending */ uint8_t *value = NULL; size_t valueLen = 0; @@ -928,12 +1078,28 @@ JNIEXPORT jbyteArray JNICALL Java_com_tidesdb_Transaction_nativeGet(JNIEnv *env, if (result != TDB_SUCCESS) { - throwTidesDBException(env, result, getErrorMessage(result)); + if (!jvm_exception_pending(env)) + throwTidesDBException(env, result, getErrorMessage(result)); + if (value != NULL) free(value); return NULL; } - jbyteArray resultArray = (*env)->NewByteArray(env, valueLen); - (*env)->SetByteArrayRegion(env, resultArray, 0, valueLen, (jbyte *)value); + if (valueLen > (size_t)JSIZE_MAX) + { + free(value); + (*env)->ThrowNew(env, (*env)->FindClass(env, "java/lang/ArrayIndexOutOfBoundsException"), + "value exceeds maximum Java array size"); + return NULL; + } + + jbyteArray resultArray = (*env)->NewByteArray(env, (jsize)valueLen); + if (resultArray == NULL) + { + free(value); + return NULL; /* JVM exception (OOM) already pending */ + } + + (*env)->SetByteArrayRegion(env, resultArray, 0, (jsize)valueLen, (jbyte *)value); free(value); return resultArray; @@ -948,15 +1114,14 @@ JNIEXPORT void JNICALL Java_com_tidesdb_Transaction_nativeDelete(JNIEnv *env, jc jsize keyLen = (*env)->GetArrayLength(env, key); jbyte *keyBytes = (*env)->GetByteArrayElements(env, key, NULL); + if (keyBytes == NULL) return; /* JVM exception already pending */ int result = tidesdb_txn_delete(txn, cf, (uint8_t *)keyBytes, keyLen); (*env)->ReleaseByteArrayElements(env, key, keyBytes, JNI_ABORT); - if (result != TDB_SUCCESS) - { + if (result != TDB_SUCCESS && !jvm_exception_pending(env)) throwTidesDBException(env, result, getErrorMessage(result)); - } } JNIEXPORT void JNICALL Java_com_tidesdb_Transaction_nativeSingleDelete(JNIEnv *env, jclass cls, @@ -968,15 +1133,14 @@ JNIEXPORT void JNICALL Java_com_tidesdb_Transaction_nativeSingleDelete(JNIEnv *e jsize keyLen = (*env)->GetArrayLength(env, key); jbyte *keyBytes = (*env)->GetByteArrayElements(env, key, NULL); + if (keyBytes == NULL) return; /* JVM exception already pending */ int result = tidesdb_txn_single_delete(txn, cf, (uint8_t *)keyBytes, keyLen); (*env)->ReleaseByteArrayElements(env, key, keyBytes, JNI_ABORT); - if (result != TDB_SUCCESS) - { + if (result != TDB_SUCCESS && !jvm_exception_pending(env)) throwTidesDBException(env, result, getErrorMessage(result)); - } } JNIEXPORT void JNICALL Java_com_tidesdb_Transaction_nativeCommit(JNIEnv *env, jclass cls, @@ -1123,15 +1287,14 @@ JNIEXPORT void JNICALL Java_com_tidesdb_TidesDBIterator_nativeSeek(JNIEnv *env, tidesdb_iter_t *iter = (tidesdb_iter_t *)(uintptr_t)handle; jsize keyLen = (*env)->GetArrayLength(env, key); jbyte *keyBytes = (*env)->GetByteArrayElements(env, key, NULL); + if (keyBytes == NULL) return; /* JVM exception already pending */ int result = tidesdb_iter_seek(iter, (uint8_t *)keyBytes, keyLen); (*env)->ReleaseByteArrayElements(env, key, keyBytes, JNI_ABORT); - if (result != TDB_SUCCESS) - { + if (result != TDB_SUCCESS && !jvm_exception_pending(env)) throwTidesDBException(env, result, getErrorMessage(result)); - } } JNIEXPORT void JNICALL Java_com_tidesdb_TidesDBIterator_nativeSeekForPrev(JNIEnv *env, jclass cls, @@ -1141,15 +1304,14 @@ JNIEXPORT void JNICALL Java_com_tidesdb_TidesDBIterator_nativeSeekForPrev(JNIEnv tidesdb_iter_t *iter = (tidesdb_iter_t *)(uintptr_t)handle; jsize keyLen = (*env)->GetArrayLength(env, key); jbyte *keyBytes = (*env)->GetByteArrayElements(env, key, NULL); + if (keyBytes == NULL) return; /* JVM exception already pending */ int result = tidesdb_iter_seek_for_prev(iter, (uint8_t *)keyBytes, keyLen); (*env)->ReleaseByteArrayElements(env, key, keyBytes, JNI_ABORT); - if (result != TDB_SUCCESS) - { + if (result != TDB_SUCCESS && !jvm_exception_pending(env)) throwTidesDBException(env, result, getErrorMessage(result)); - } } JNIEXPORT jboolean JNICALL Java_com_tidesdb_TidesDBIterator_nativeValid(JNIEnv *env, jclass cls, @@ -1195,12 +1357,22 @@ JNIEXPORT jbyteArray JNICALL Java_com_tidesdb_TidesDBIterator_nativeKey(JNIEnv * int result = tidesdb_iter_key(iter, &key, &keyLen); if (result != TDB_SUCCESS) { - throwTidesDBException(env, result, getErrorMessage(result)); + if (!jvm_exception_pending(env)) + throwTidesDBException(env, result, getErrorMessage(result)); + return NULL; + } + + if (keyLen > (size_t)JSIZE_MAX) + { + (*env)->ThrowNew(env, (*env)->FindClass(env, "java/lang/ArrayIndexOutOfBoundsException"), + "key exceeds maximum Java array size"); return NULL; } - jbyteArray resultArray = (*env)->NewByteArray(env, keyLen); - (*env)->SetByteArrayRegion(env, resultArray, 0, keyLen, (jbyte *)key); + jbyteArray resultArray = (*env)->NewByteArray(env, (jsize)keyLen); + if (resultArray == NULL) return NULL; /* JVM exception (OOM) already pending */ + + (*env)->SetByteArrayRegion(env, resultArray, 0, (jsize)keyLen, (jbyte *)key); return resultArray; } @@ -1215,12 +1387,22 @@ JNIEXPORT jbyteArray JNICALL Java_com_tidesdb_TidesDBIterator_nativeValue(JNIEnv int result = tidesdb_iter_value(iter, &value, &valueLen); if (result != TDB_SUCCESS) { - throwTidesDBException(env, result, getErrorMessage(result)); + if (!jvm_exception_pending(env)) + throwTidesDBException(env, result, getErrorMessage(result)); return NULL; } - jbyteArray resultArray = (*env)->NewByteArray(env, valueLen); - (*env)->SetByteArrayRegion(env, resultArray, 0, valueLen, (jbyte *)value); + if (valueLen > (size_t)JSIZE_MAX) + { + (*env)->ThrowNew(env, (*env)->FindClass(env, "java/lang/ArrayIndexOutOfBoundsException"), + "value exceeds maximum Java array size"); + return NULL; + } + + jbyteArray resultArray = (*env)->NewByteArray(env, (jsize)valueLen); + if (resultArray == NULL) return NULL; /* JVM exception (OOM) already pending */ + + (*env)->SetByteArrayRegion(env, resultArray, 0, (jsize)valueLen, (jbyte *)value); return resultArray; } @@ -1525,20 +1707,18 @@ JNIEXPORT jobject JNICALL Java_com_tidesdb_TidesDB_nativeGetDbStats(JNIEnv *env, } jclass dbStatsClass = (*env)->FindClass(env, "com/tidesdb/DbStats"); - /* Constructor signature: - int, long, long, long, int, int, long, int, int, long, int, long, long, long, long, - boolean, long, int, boolean, int, long, - boolean, String, long, long, int, long, long, long, long, boolean, - long (primaryEpoch), long (seenEpoch), - long, long, long, long, long, long, long, long */ + if (dbStatsClass == NULL) return NULL; + jmethodID constructor = (*env)->GetMethodID(env, dbStatsClass, "", "(IJJJIIJIIJIJJJJZJIZIJZLjava/lang/String;JJIJJJJZJJJJJJJJJJ)V"); + if (constructor == NULL) return NULL; jstring connectorStr = NULL; if (db_stats.object_store_connector != NULL) { connectorStr = (*env)->NewStringUTF(env, db_stats.object_store_connector); + if (connectorStr == NULL) return NULL; } return (*env)->NewObject( @@ -1575,7 +1755,14 @@ JNIEXPORT jdouble JNICALL Java_com_tidesdb_ColumnFamily_nativeRangeCost(JNIEnv * jsize keyBLen = (*env)->GetArrayLength(env, keyB); jbyte *keyABytes = (*env)->GetByteArrayElements(env, keyA, NULL); + if (keyABytes == NULL) return 0.0; /* JVM exception already pending */ + jbyte *keyBBytes = (*env)->GetByteArrayElements(env, keyB, NULL); + if (keyBBytes == NULL) + { + (*env)->ReleaseByteArrayElements(env, keyA, keyABytes, JNI_ABORT); + return 0.0; /* JVM exception already pending */ + } double cost = 0.0; int result = @@ -1586,7 +1773,8 @@ JNIEXPORT jdouble JNICALL Java_com_tidesdb_ColumnFamily_nativeRangeCost(JNIEnv * if (result != TDB_SUCCESS) { - throwTidesDBException(env, result, getErrorMessage(result)); + if (!jvm_exception_pending(env)) + throwTidesDBException(env, result, getErrorMessage(result)); return 0.0; } @@ -1826,18 +2014,40 @@ JNIEXPORT jobject JNICALL Java_com_tidesdb_TidesDBIterator_nativeKeyValue(JNIEnv int result = tidesdb_iter_key_value(iter, &key, &keyLen, &value, &valueLen); if (result != TDB_SUCCESS) { - throwTidesDBException(env, result, getErrorMessage(result)); + if (!jvm_exception_pending(env)) + throwTidesDBException(env, result, getErrorMessage(result)); + return NULL; + } + + if (keyLen > (size_t)JSIZE_MAX) + { + (*env)->ThrowNew(env, (*env)->FindClass(env, "java/lang/ArrayIndexOutOfBoundsException"), + "key exceeds maximum Java array size"); return NULL; } - jbyteArray jkey = (*env)->NewByteArray(env, keyLen); - (*env)->SetByteArrayRegion(env, jkey, 0, keyLen, (jbyte *)key); + if (valueLen > (size_t)JSIZE_MAX) + { + (*env)->ThrowNew(env, (*env)->FindClass(env, "java/lang/ArrayIndexOutOfBoundsException"), + "value exceeds maximum Java array size"); + return NULL; + } + + jbyteArray jkey = (*env)->NewByteArray(env, (jsize)keyLen); + if (jkey == NULL) return NULL; /* JVM exception (OOM) already pending */ - jbyteArray jvalue = (*env)->NewByteArray(env, valueLen); - (*env)->SetByteArrayRegion(env, jvalue, 0, valueLen, (jbyte *)value); + jbyteArray jvalue = (*env)->NewByteArray(env, (jsize)valueLen); + if (jvalue == NULL) return NULL; /* JVM exception (OOM) already pending; jkey will be GCed */ + + (*env)->SetByteArrayRegion(env, jkey, 0, (jsize)keyLen, (jbyte *)key); + (*env)->SetByteArrayRegion(env, jvalue, 0, (jsize)valueLen, (jbyte *)value); jclass kvClass = (*env)->FindClass(env, "com/tidesdb/KeyValue"); + if (kvClass == NULL) return NULL; + jmethodID ctor = (*env)->GetMethodID(env, kvClass, "", "([B[B)V"); + if (ctor == NULL) return NULL; - return (*env)->NewObject(env, kvClass, ctor, jkey, jvalue); + jobject result_obj = (*env)->NewObject(env, kvClass, ctor, jkey, jvalue); + return result_obj; } diff --git a/src/test/java/com/tidesdb/TidesDBTest.java b/src/test/java/com/tidesdb/TidesDBTest.java index d4b7787..6e98f1a 100644 --- a/src/test/java/com/tidesdb/TidesDBTest.java +++ b/src/test/java/com/tidesdb/TidesDBTest.java @@ -2646,4 +2646,140 @@ void testSetClearHookNotRacingClose() throws TidesDBException, InterruptedExcept replacer.join(5000); } } + + @Test + @Order(70) + void testJniBufferMethodsCoverage() throws TidesDBException { + Config config = Config.builder(tempDir.resolve("testdb_coverage").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + try (TidesDB db = TidesDB.open(config)) { + ColumnFamilyConfig cfConfig = ColumnFamilyConfig.defaultConfig(); + db.createColumnFamily("coverage_cf", cfConfig); + + ColumnFamily cf = db.getColumnFamily("coverage_cf"); + + // Insert 100 entries into one transaction and commit + try (Transaction txn = db.beginTransaction()) { + for (int i = 0; i < 100; i++) { + byte[] key = String.format("cov_key%04d", i).getBytes(StandardCharsets.UTF_8); + byte[] value = ("cov_value" + i).getBytes(StandardCharsets.UTF_8); + txn.put(cf, key, value); + } + txn.commit(); + } + + // 1. Transaction put/get/delete/singleDelete with various byte arrays + try (Transaction txn = db.beginTransaction()) { + // put with a 0-length value and a 1-byte key + byte[] tinyKey = new byte[]{0x42}; + byte[] emptyValue = new byte[0]; + txn.put(cf, tinyKey, emptyValue); + + // get of that entry + byte[] got = txn.get(cf, tinyKey); + assertNotNull(got, "get should return a non-null result for the 0-length value entry"); + assertEquals(0, got.length, "value should be 0-length"); + + // delete that entry + txn.delete(cf, tinyKey); + + // singleDelete of a fresh entry + byte[] sdKey = new byte[]{0x43}; + byte[] sdValue = new byte[]{0x01}; + txn.put(cf, sdKey, sdValue); + txn.singleDelete(cf, sdKey); + + txn.commit(); + } + + // Confirm get of deleted key throws TidesDBException + try (Transaction txn = db.beginTransaction()) { + assertThrows(TidesDBException.class, () -> txn.get(cf, new byte[]{0x42})); + assertThrows(TidesDBException.class, () -> txn.get(cf, new byte[]{0x43})); + } + + // 2. Iterator seek/seekForPrev/key/value/keyValue + try (Transaction txn = db.beginTransaction()) { + try (TidesDBIterator iter = txn.newIterator(cf)) { + iter.seekToFirst(); + int count = 0; + while (iter.isValid()) { + byte[] k = iter.key(); + byte[] v = iter.value(); + KeyValue kv = iter.keyValue(); + assertNotNull(k, "iterator key should not be null"); + assertNotNull(v, "iterator value should not be null"); + assertNotNull(kv, "iterator keyValue should not be null"); + assertNotNull(kv.getKey(), "KeyValue.getKey() should not be null"); + assertNotNull(kv.getValue(), "KeyValue.getValue() should not be null"); + count++; + iter.next(); + } + assertEquals(100, count, "iterator should visit exactly 100 entries"); + } + } + + // seek and seekForPrev + try (Transaction txn = db.beginTransaction()) { + try (TidesDBIterator iter = txn.newIterator(cf)) { + byte[] targetKey = String.format("cov_key%04d", 50).getBytes(StandardCharsets.UTF_8); + iter.seek(targetKey); + assertTrue(iter.isValid(), "iterator should be valid after seek"); + byte[] seekedKey = iter.key(); + assertNotNull(seekedKey); + assertTrue(seekedKey.length > 0, "seeked key should not be empty"); + + iter.seekForPrev(targetKey); + assertTrue(iter.isValid(), "iterator should be valid after seekForPrev"); + byte[] seekedForPrevKey = iter.key(); + assertNotNull(seekedForPrevKey); + assertTrue(seekedForPrevKey.length > 0, "seekForPrev key should not be empty"); + } + } + + // 3. compactRange with single-byte bounds and with one-null-one-nonnull + cf.flushMemtable(); + assertDoesNotThrow(() -> cf.compactRange(new byte[]{0x10}, new byte[]{0x20}), + "compactRange with valid bounds should succeed"); + assertDoesNotThrow(() -> cf.compactRange(null, new byte[]{0x20}), + "compactRange with null start should succeed"); + assertDoesNotThrow(() -> cf.compactRange(new byte[]{0x10}, null), + "compactRange with null end should succeed"); + + // 4. rangeCost with single-byte bounds + double cost = cf.rangeCost(new byte[]{0x01}, new byte[]{(byte) 0xFF}); + assertTrue(cost >= 0.0, "rangeCost should be non-negative"); + + // 5. getStats on the CF after data insertion + Stats stats = cf.getStats(); + assertNotNull(stats, "getStats should return non-null"); + + // 6. getDbStats on the database handle + DbStats dbStats = db.getDbStats(); + assertNotNull(dbStats, "getDbStats should return non-null"); + + // 7. listColumnFamilies and iterate the result + String[] families = db.listColumnFamilies(); + assertNotNull(families, "listColumnFamilies should return non-null"); + assertTrue(families.length > 0, "should have at least one column family"); + boolean found = false; + for (String f : families) { + if ("coverage_cf".equals(f)) { + found = true; + break; + } + } + assertTrue(found, "coverage_cf should be in the list"); + + // 8. getCacheStats on the database handle + CacheStats cacheStats = db.getCacheStats(); + assertNotNull(cacheStats, "getCacheStats should return non-null"); + } + } } From bd86a85d4925c82ddd4a1b26782bd82d2933ecd5 Mon Sep 17 00:00:00 2001 From: dhoard Date: Sun, 12 Jul 2026 23:13:02 -0400 Subject: [PATCH 5/7] fix: Fixed filesystem object store failback Signed-off-by: dhoard --- src/main/c/com_tidesdb_TidesDB.c | 24 +++++++ src/test/java/com/tidesdb/TidesDBTest.java | 82 ++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/src/main/c/com_tidesdb_TidesDB.c b/src/main/c/com_tidesdb_TidesDB.c index 87af5a0..fec33e6 100644 --- a/src/main/c/com_tidesdb_TidesDB.c +++ b/src/main/c/com_tidesdb_TidesDB.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #ifndef _WIN32 #include @@ -156,7 +157,30 @@ JNIEXPORT jlong JNICALL Java_com_tidesdb_TidesDB_nativeOpen( } if (fs_path != NULL) { + /* Validate that the path is an existing directory before creating the + * connector. tidesdb_objstore_fs_create silently succeeds on a regular + * file path, which would cause the database to open with a broken + * object store backend. */ + struct stat st; + if (stat(fs_path, &st) != 0 || !S_ISDIR(st.st_mode)) + { + (*env)->ReleaseStringUTFChars(env, objectStoreFsPath, fs_path); + (*env)->ReleaseStringUTFChars(env, dbPath, path); + if (!jvm_exception_pending(env)) + throwTidesDBException(env, TDB_ERR_IO, + "Failed to create filesystem object store connector"); + return 0; + } obj_store = tidesdb_objstore_fs_create(fs_path); + if (obj_store == NULL) + { + (*env)->ReleaseStringUTFChars(env, objectStoreFsPath, fs_path); + (*env)->ReleaseStringUTFChars(env, dbPath, path); + if (!jvm_exception_pending(env)) + throwTidesDBException(env, TDB_ERR_IO, + "Failed to create filesystem object store connector"); + return 0; + } } } diff --git a/src/test/java/com/tidesdb/TidesDBTest.java b/src/test/java/com/tidesdb/TidesDBTest.java index 6e98f1a..5f8963b 100644 --- a/src/test/java/com/tidesdb/TidesDBTest.java +++ b/src/test/java/com/tidesdb/TidesDBTest.java @@ -22,6 +22,7 @@ import org.junit.jupiter.api.io.TempDir; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.time.Instant; import java.util.ArrayList; @@ -2647,6 +2648,87 @@ void testSetClearHookNotRacingClose() throws TidesDBException, InterruptedExcept } } + @Test + @Order(71) + void testOpenWithObjectStoreFsPathFailsOnBadPath() throws Exception { + // A regular file (not a directory) as objectStoreFsPath must cause tidesdb_objstore_fs_create + // to return NULL, which should surface as TidesDBException instead of silently opening + // an ordinary local database. + Path osFile = tempDir.resolve("os_file"); + Files.createFile(osFile); + + Config config = Config.builder(tempDir.resolve("testdb_fs_badpath").toString()) + .objectStoreFsPath(osFile.toString()) + .build(); + + TidesDBException ex = assertThrows(TidesDBException.class, () -> TidesDB.open(config)); + assertEquals(TidesDBException.ERR_IO, ex.getErrorCode()); + } + + @Test + @Order(72) + void testOpenWithObjectStoreFsPathSucceedsWithValidDirectory() throws TidesDBException { + Path osDir = tempDir.resolve("os_dir"); + osDir.toFile().mkdirs(); + + Config config = Config.builder(tempDir.resolve("testdb_fs_goodpath").toString()) + .objectStoreFsPath(osDir.toString()) + .build(); + + try (TidesDB db = TidesDB.open(config)) { + assertNotNull(db); + DbStats dbStats = db.getDbStats(); + assertNotNull(dbStats); + assertTrue(dbStats.isObjectStoreEnabled()); + } + } + + @Test + @Order(73) + void testOpenWithoutObjectStoreFsPathSucceeds() throws TidesDBException { + Config config = Config.builder(tempDir.resolve("plaindb").toString()) + .numFlushThreads(2) + .numCompactionThreads(2) + .logLevel(LogLevel.INFO) + .blockCacheSize(64 * 1024 * 1024) + .maxOpenSSTables(256) + .build(); + + try (TidesDB db = TidesDB.open(config)) { + assertNotNull(db); + } + } + + @Test + @Order(74) + void testS3PrecedenceOverFsPathOnFailure() throws TidesDBException { + Path osFile = tempDir.resolve("os_file_s3"); + try { + Files.createFile(osFile); + } catch (java.io.IOException e) { + throw new RuntimeException(e); + } + + S3Config s3 = S3Config.builder() + .endpoint("127.0.0.1:19001") + .bucket("tidesdb-test") + .accessKey("minioadmin") + .secretKey("minioadmin") + .usePathStyle(true) + .useSsl(false) + .build(); + + Config config = Config.builder(tempDir.resolve("testdb_s3_over_fs").toString()) + .objectStoreS3Config(s3) + .objectStoreFsPath(osFile.toString()) + .build(); + + // S3 takes precedence over fs path. If S3 is available, the S3 connector creation or + // open fails with TidesDBException. If S3 is unavailable, the S3 connector creation + // itself throws. In neither case does a silent filesystem fallback occur. + assertThrows(TidesDBException.class, () -> TidesDB.open(config)); + } + @Test @Order(70) void testJniBufferMethodsCoverage() throws TidesDBException { From 60488cbdaa84069c44dcad3eaa6b1de72a3a4b2e Mon Sep 17 00:00:00 2001 From: dhoard Date: Sun, 12 Jul 2026 23:25:40 -0400 Subject: [PATCH 6/7] fix: Fixed configuration validation Signed-off-by: dhoard --- .../java/com/tidesdb/ColumnFamilyConfig.java | 68 ++++++ src/main/java/com/tidesdb/Config.java | 25 +++ .../java/com/tidesdb/ObjectStoreConfig.java | 25 +++ src/main/java/com/tidesdb/S3Config.java | 6 + src/test/java/com/tidesdb/TidesDBTest.java | 206 ++++++++++++++++++ 5 files changed, 330 insertions(+) diff --git a/src/main/java/com/tidesdb/ColumnFamilyConfig.java b/src/main/java/com/tidesdb/ColumnFamilyConfig.java index ff7c4a6..6a7a017 100644 --- a/src/main/java/com/tidesdb/ColumnFamilyConfig.java +++ b/src/main/java/com/tidesdb/ColumnFamilyConfig.java @@ -420,7 +420,75 @@ public Builder objectPrefetchCompaction(boolean objectPrefetchCompaction) { } public ColumnFamilyConfig build() { + validate(); return new ColumnFamilyConfig(this); } + + private void validate() { + // Nullable-enum checks (do first, before field-level checks) + if (compressionAlgorithm == null) { + throw new IllegalArgumentException("compressionAlgorithm must not be null"); + } + if (syncMode == null) { + throw new IllegalArgumentException("syncMode must not be null"); + } + if (defaultIsolationLevel == null) { + throw new IllegalArgumentException("defaultIsolationLevel must not be null"); + } + + // Non-negative (zero sentinel OK) + if (klogValueThreshold < 0) { + throw new IllegalArgumentException("klogValueThreshold must not be negative, was: " + klogValueThreshold); + } + if (syncIntervalUs < 0) { + throw new IllegalArgumentException("syncIntervalUs must not be negative, was: " + syncIntervalUs); + } + if (minDiskSpace < 0) { + throw new IllegalArgumentException("minDiskSpace must not be negative, was: " + minDiskSpace); + } + if (dividingLevelOffset < 0) { + throw new IllegalArgumentException("dividingLevelOffset must not be negative, was: " + dividingLevelOffset); + } + if (blockIndexPrefixLen < 0) { + throw new IllegalArgumentException("blockIndexPrefixLen must not be negative, was: " + blockIndexPrefixLen); + } + if (skipListMaxLevel < 0) { + throw new IllegalArgumentException("skipListMaxLevel must not be negative, was: " + skipListMaxLevel); + } + + // Positive-required (zero rejected) + if (writeBufferSize <= 0) { + throw new IllegalArgumentException("writeBufferSize must be positive, was: " + writeBufferSize); + } + if (levelSizeRatio <= 0) { + throw new IllegalArgumentException("levelSizeRatio must be positive, was: " + levelSizeRatio); + } + if (minLevels <= 0) { + throw new IllegalArgumentException("minLevels must be positive, was: " + minLevels); + } + if (indexSampleRatio <= 0) { + throw new IllegalArgumentException("indexSampleRatio must be positive, was: " + indexSampleRatio); + } + if (l1FileCountTrigger <= 0) { + throw new IllegalArgumentException("l1FileCountTrigger must be positive, was: " + l1FileCountTrigger); + } + if (l0QueueStallThreshold <= 0) { + throw new IllegalArgumentException("l0QueueStallThreshold must be positive, was: " + l0QueueStallThreshold); + } + if (tombstoneDensityMinEntries <= 0) { + throw new IllegalArgumentException("tombstoneDensityMinEntries must be positive, was: " + tombstoneDensityMinEntries); + } + + // Float/double range and finiteness + if (Double.isNaN(bloomFPR) || Double.isInfinite(bloomFPR) || bloomFPR < 0.0 || bloomFPR > 1.0) { + throw new IllegalArgumentException("bloomFPR must be finite and in [0.0, 1.0], was: " + bloomFPR); + } + if (Float.isNaN(skipListProbability) || Float.isInfinite(skipListProbability) || skipListProbability < 0.0f || skipListProbability > 1.0f) { + throw new IllegalArgumentException("skipListProbability must be finite and in [0.0, 1.0], was: " + skipListProbability); + } + if (Double.isNaN(tombstoneDensityTrigger) || Double.isInfinite(tombstoneDensityTrigger) || tombstoneDensityTrigger < 0.0 || tombstoneDensityTrigger > 1.0) { + throw new IllegalArgumentException("tombstoneDensityTrigger must be finite and in [0.0, 1.0], was: " + tombstoneDensityTrigger); + } + } } } diff --git a/src/main/java/com/tidesdb/Config.java b/src/main/java/com/tidesdb/Config.java index 16d9d2c..a2ddbcb 100644 --- a/src/main/java/com/tidesdb/Config.java +++ b/src/main/java/com/tidesdb/Config.java @@ -366,6 +366,31 @@ private void validate() throws IllegalArgumentException { if (maxOpenSSTables <= 0) { throw new IllegalArgumentException("Max open SSTables must be positive"); } + if (logTruncationAt < 0) { + throw new IllegalArgumentException("logTruncationAt must not be negative, was: " + logTruncationAt); + } + if (maxMemoryUsage < 0) { + throw new IllegalArgumentException("maxMemoryUsage must not be negative, was: " + maxMemoryUsage); + } + if (unifiedMemtableWriteBufferSize < 0) { + throw new IllegalArgumentException("unifiedMemtableWriteBufferSize must not be negative, was: " + unifiedMemtableWriteBufferSize); + } + if (unifiedMemtableSyncIntervalUs < 0) { + throw new IllegalArgumentException("unifiedMemtableSyncIntervalUs must not be negative, was: " + unifiedMemtableSyncIntervalUs); + } + if (unifiedMemtableSkipListMaxLevel < 0) { + throw new IllegalArgumentException("unifiedMemtableSkipListMaxLevel must not be negative, was: " + unifiedMemtableSkipListMaxLevel); + } + if (Float.isNaN(unifiedMemtableSkipListProbability) || Float.isInfinite(unifiedMemtableSkipListProbability) + || unifiedMemtableSkipListProbability < 0.0f || unifiedMemtableSkipListProbability > 1.0f) { + throw new IllegalArgumentException("unifiedMemtableSkipListProbability must be finite and in [0.0, 1.0], was: " + unifiedMemtableSkipListProbability); + } + if (unifiedMemtableSyncMode < 0) { + throw new IllegalArgumentException("unifiedMemtableSyncMode must not be negative, was: " + unifiedMemtableSyncMode); + } + if (maxConcurrentFlushes < 0) { + throw new IllegalArgumentException("maxConcurrentFlushes must not be negative, was: " + maxConcurrentFlushes); + } } } } diff --git a/src/main/java/com/tidesdb/ObjectStoreConfig.java b/src/main/java/com/tidesdb/ObjectStoreConfig.java index 1e89bbd..2dff8fd 100644 --- a/src/main/java/com/tidesdb/ObjectStoreConfig.java +++ b/src/main/java/com/tidesdb/ObjectStoreConfig.java @@ -196,7 +196,32 @@ public Builder replicaReplayWal(boolean replicaReplayWal) { } public ObjectStoreConfig build() { + validate(); return new ObjectStoreConfig(this); } + + private void validate() { + if (localCacheMaxBytes < 0) { + throw new IllegalArgumentException("localCacheMaxBytes must not be negative, was: " + localCacheMaxBytes); + } + if (maxConcurrentUploads <= 0) { + throw new IllegalArgumentException("maxConcurrentUploads must be positive, was: " + maxConcurrentUploads); + } + if (maxConcurrentDownloads <= 0) { + throw new IllegalArgumentException("maxConcurrentDownloads must be positive, was: " + maxConcurrentDownloads); + } + if (multipartThreshold < 0) { + throw new IllegalArgumentException("multipartThreshold must not be negative, was: " + multipartThreshold); + } + if (multipartPartSize < 0) { + throw new IllegalArgumentException("multipartPartSize must not be negative, was: " + multipartPartSize); + } + if (walSyncThresholdBytes < 0) { + throw new IllegalArgumentException("walSyncThresholdBytes must not be negative, was: " + walSyncThresholdBytes); + } + if (replicaSyncIntervalUs < 0) { + throw new IllegalArgumentException("replicaSyncIntervalUs must not be negative, was: " + replicaSyncIntervalUs); + } + } } } diff --git a/src/main/java/com/tidesdb/S3Config.java b/src/main/java/com/tidesdb/S3Config.java index 029608f..4b070da 100644 --- a/src/main/java/com/tidesdb/S3Config.java +++ b/src/main/java/com/tidesdb/S3Config.java @@ -157,6 +157,12 @@ public S3Config build() { if (secretKey == null || secretKey.isEmpty()) { throw new IllegalArgumentException("S3 secret key is required"); } + if (multipartThreshold < 0) { + throw new IllegalArgumentException("multipartThreshold must not be negative, was: " + multipartThreshold); + } + if (multipartPartSize < 0) { + throw new IllegalArgumentException("multipartPartSize must not be negative, was: " + multipartPartSize); + } return new S3Config(this); } } diff --git a/src/test/java/com/tidesdb/TidesDBTest.java b/src/test/java/com/tidesdb/TidesDBTest.java index 5f8963b..8cff846 100644 --- a/src/test/java/com/tidesdb/TidesDBTest.java +++ b/src/test/java/com/tidesdb/TidesDBTest.java @@ -2699,6 +2699,212 @@ void testOpenWithoutObjectStoreFsPathSucceeds() throws TidesDBException { } } + @Test + @Order(75) + void testConfigValidationRejectsNegativeUnsignedFields() { + // logTruncationAt < 0 + assertThrows(IllegalArgumentException.class, + () -> Config.builder(tempDir.toString()).logTruncationAt(-1).build()); + try { + Config.builder(tempDir.toString()).logTruncationAt(-1).build(); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("logTruncationAt"), + "message should mention field name, was: " + e.getMessage()); + assertTrue(e.getMessage().toLowerCase().contains("negative"), + "message should mention negative, was: " + e.getMessage()); + } + + // maxMemoryUsage < 0 + assertThrows(IllegalArgumentException.class, + () -> Config.builder(tempDir.toString()).maxMemoryUsage(-1).build()); + try { + Config.builder(tempDir.toString()).maxMemoryUsage(-1).build(); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("maxMemoryUsage"), + "message should mention field name, was: " + e.getMessage()); + assertTrue(e.getMessage().toLowerCase().contains("negative"), + "message should mention negative, was: " + e.getMessage()); + } + + // unifiedMemtableWriteBufferSize < 0 + assertThrows(IllegalArgumentException.class, + () -> Config.builder(tempDir.toString()).unifiedMemtableWriteBufferSize(-1).build()); + try { + Config.builder(tempDir.toString()).unifiedMemtableWriteBufferSize(-1).build(); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("unifiedMemtableWriteBufferSize"), + "message should mention field name, was: " + e.getMessage()); + assertTrue(e.getMessage().toLowerCase().contains("negative"), + "message should mention negative, was: " + e.getMessage()); + } + + // unifiedMemtableSyncIntervalUs < 0 + assertThrows(IllegalArgumentException.class, + () -> Config.builder(tempDir.toString()).unifiedMemtableSyncIntervalUs(-1).build()); + try { + Config.builder(tempDir.toString()).unifiedMemtableSyncIntervalUs(-1).build(); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("unifiedMemtableSyncIntervalUs"), + "message should mention field name, was: " + e.getMessage()); + assertTrue(e.getMessage().toLowerCase().contains("negative"), + "message should mention negative, was: " + e.getMessage()); + } + } + + @Test + @Order(76) + void testObjectStoreConfigValidation() { + // Reject negative unsigned-native fields (zero sentinel accepted) + assertThrows(IllegalArgumentException.class, + () -> ObjectStoreConfig.builder().localCacheMaxBytes(-1).build()); + assertThrows(IllegalArgumentException.class, + () -> ObjectStoreConfig.builder().multipartThreshold(-1).build()); + assertThrows(IllegalArgumentException.class, + () -> ObjectStoreConfig.builder().multipartPartSize(-1).build()); + assertThrows(IllegalArgumentException.class, + () -> ObjectStoreConfig.builder().walSyncThresholdBytes(-1).build()); + assertThrows(IllegalArgumentException.class, + () -> ObjectStoreConfig.builder().replicaSyncIntervalUs(-1).build()); + + // Positive-required (<= 0 rejected) + assertThrows(IllegalArgumentException.class, + () -> ObjectStoreConfig.builder().maxConcurrentUploads(0).build()); + try { + ObjectStoreConfig.builder().maxConcurrentUploads(0).build(); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("maxConcurrentUploads"), + "message should mention field name, was: " + e.getMessage()); + assertTrue(e.getMessage().toLowerCase().contains("positive"), + "message should mention positive, was: " + e.getMessage()); + } + + assertThrows(IllegalArgumentException.class, + () -> ObjectStoreConfig.builder().maxConcurrentDownloads(-1).build()); + try { + ObjectStoreConfig.builder().maxConcurrentDownloads(-1).build(); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("maxConcurrentDownloads"), + "message should mention field name, was: " + e.getMessage()); + assertTrue(e.getMessage().toLowerCase().contains("positive"), + "message should mention positive, was: " + e.getMessage()); + } + + // Zero sentinel accepted for localCacheMaxBytes + assertDoesNotThrow(() -> ObjectStoreConfig.builder().localCacheMaxBytes(0).build()); + } + + @Test + @Order(77) + void testS3ConfigValidation() { + S3Config.Builder baseBuilder = S3Config.builder() + .endpoint("s3.amazonaws.com") + .bucket("b") + .accessKey("ak") + .secretKey("sk"); + + // Reject negative multipart fields + assertThrows(IllegalArgumentException.class, + () -> baseBuilder.multipartThreshold(-1).build()); + assertThrows(IllegalArgumentException.class, + () -> S3Config.builder() + .endpoint("s3.amazonaws.com") + .bucket("b") + .accessKey("ak") + .secretKey("sk") + .multipartPartSize(-1) + .build()); + + // Zero sentinels accepted + assertDoesNotThrow(() -> S3Config.builder() + .endpoint("s3.amazonaws.com") + .bucket("b") + .accessKey("ak") + .secretKey("sk") + .multipartThreshold(0) + .multipartPartSize(0) + .build()); + + // Existing required-string validation still passes + assertThrows(IllegalArgumentException.class, () -> S3Config.builder().build()); + } + + @Test + @Order(78) + void testColumnFamilyConfigValidation() { + // Reject negative unsigned-native fields (zero sentinel accepted) + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().klogValueThreshold(-1).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().syncIntervalUs(-1).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().minDiskSpace(-1).build()); + + // Positive-required fields (<= 0 rejected) + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().writeBufferSize(0).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().levelSizeRatio(0).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().minLevels(0).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().indexSampleRatio(0).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().l1FileCountTrigger(0).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().l0QueueStallThreshold(0).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().tombstoneDensityMinEntries(0).build()); + + // Non-negative-int fields (zero acceptable, negative rejected) + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().dividingLevelOffset(-1).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().blockIndexPrefixLen(-1).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().skipListMaxLevel(-1).build()); + + // Float/double NaN/infinity/range rejection: bloomFPR + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().bloomFPR(Double.NaN).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().bloomFPR(Double.POSITIVE_INFINITY).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().bloomFPR(-0.1).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().bloomFPR(1.1).build()); + + // Float/double NaN/infinity/range rejection: skipListProbability + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().skipListProbability(Float.NaN).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().skipListProbability(Float.POSITIVE_INFINITY).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().skipListProbability(-0.1f).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().skipListProbability(1.1f).build()); + + // Float/double NaN/infinity/range rejection: tombstoneDensityTrigger + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().tombstoneDensityTrigger(Double.NaN).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().tombstoneDensityTrigger(Double.POSITIVE_INFINITY).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().tombstoneDensityTrigger(-0.1).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().tombstoneDensityTrigger(1.1).build()); + + // Nullable-enum null rejection + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().compressionAlgorithm(null).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().syncMode(null).build()); + assertThrows(IllegalArgumentException.class, + () -> ColumnFamilyConfig.builder().defaultIsolationLevel(null).build()); + + // fromNative compatibility: engine-supplied values must be accepted + assertDoesNotThrow(() -> ColumnFamilyConfig.defaultConfig()); + } + @Test @Order(74) void testS3PrecedenceOverFsPathOnFailure() throws TidesDBException { From 65d8426614894695b8c52594f538032aa5e6bb48 Mon Sep 17 00:00:00 2001 From: dhoard Date: Sun, 12 Jul 2026 23:50:18 -0400 Subject: [PATCH 7/7] chore: Code coverage Signed-off-by: dhoard --- pom.xml | 57 +- .../java/com/tidesdb/PojoAndEnumTest.java | 842 ++++++++++++++++++ src/test/java/com/tidesdb/TidesDBTest.java | 277 ++++++ 3 files changed, 1167 insertions(+), 9 deletions(-) create mode 100644 src/test/java/com/tidesdb/PojoAndEnumTest.java diff --git a/pom.xml b/pom.xml index 45d61e7..d8c6fdf 100644 --- a/pom.xml +++ b/pom.xml @@ -36,9 +36,23 @@ - UTF-8 + + false 11 + UTF-8 + + + 3.27.7 5.10.1 + + 0.8.12 + 3.15.0 + 3.6.3 + 3.5.0 + 3.12.0 + 3.4.0 + 3.5.6 + 2.21.0 @@ -54,7 +68,7 @@ org.assertj assertj-core - 3.27.7 + ${assertj-core.version} test @@ -65,7 +79,7 @@ org.apache.maven.plugins maven-compiler-plugin - 3.15.0 + ${maven-compiler-plugin.version} ${maven.compiler.release} @@ -75,7 +89,7 @@ org.codehaus.mojo versions-maven-plugin - 2.21.0 + ${versions-maven-plugin.version} file://${project.basedir}/version-rules.xml @@ -85,7 +99,7 @@ org.apache.maven.plugins maven-enforcer-plugin - 3.6.3 + ${maven-enforcer-plugin.version} enforce-plugin-versions @@ -106,25 +120,50 @@ + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + + org.apache.maven.plugins maven-surefire-plugin - 3.5.6 + ${maven-surefire-plugin.version} + + ${argLine} ${test.jvm.args} + org.apache.maven.plugins maven-jar-plugin - 3.5.0 + ${maven-jar-plugin.version} org.apache.maven.plugins maven-source-plugin - 3.4.0 + ${maven-source-plugin.version} attach-sources @@ -139,7 +178,7 @@ org.apache.maven.plugins maven-javadoc-plugin - 3.12.0 + ${maven-javadoc-plugin.version}