From 6ed7af02b7c3893d31cf624480d12e7f844e815f Mon Sep 17 00:00:00 2001 From: Narasimha-sc <166327228+Narasimha-sc@users.noreply.github.com> Date: Sat, 26 Sep 2026 11:22:12 +0000 Subject: [PATCH] android, desktop: free strings returned by the core The core allocates every chat_* response with malloc and passes ownership to the caller, as iOS does in API.swift. Both JNI bindings copied the bytes into a Java string and dropped the pointer, so every command, event and file read leaked its whole response into the C allocator - invisible to both the JVM and the Haskell heap profiler. Desktop frees in decode_to_utf8_string, the funnel every entry point already goes through. Android has no such funnel because NewStringUTF is called inline, so decode_and_free is added and the call sites routed through it. chatReadFile frees directly on both. --- .../src/commonMain/cpp/android/simplex-api.c | 41 ++++--- .../src/commonMain/cpp/desktop/simplex-api.c | 2 + plans/2026-09-26-free-core-strings-in-jni.md | 103 ++++++++++++++++++ 3 files changed, 131 insertions(+), 15 deletions(-) create mode 100644 plans/2026-09-26-free-core-strings-in-jni.md diff --git a/apps/multiplatform/common/src/commonMain/cpp/android/simplex-api.c b/apps/multiplatform/common/src/commonMain/cpp/android/simplex-api.c index c0497daacb..f66fd82473 100644 --- a/apps/multiplatform/common/src/commonMain/cpp/android/simplex-api.c +++ b/apps/multiplatform/common/src/commonMain/cpp/android/simplex-api.c @@ -4,6 +4,9 @@ //#include //#include +// from libc (stdlib.h is not included because of the reallocarray stub below) +void free(void *ptr); + // from the RTS void hs_init_with_rtsopts(int * argc, char **argv[]); @@ -49,6 +52,13 @@ Java_chat_simplex_common_platform_CoreKt_initHS(__unused JNIEnv *env, __unused j // from simplex-chat typedef long* chat_ctrl; +// Strings returned by chat_* functions are allocated with malloc and owned by the caller. +static jstring decode_and_free(JNIEnv *env, char *string) { + jstring res = (*env)->NewStringUTF(env, string); + free(string); + return res; +} + /* When you start using any new function from Haskell libraries, you have to add the function name to the file libsimplex.dll.def in the root directory. @@ -80,7 +90,7 @@ Java_chat_simplex_common_platform_CoreKt_chatMigrateInit(JNIEnv *env, __unused j const char *_dbKey = (*env)->GetStringUTFChars(env, dbKey, JNI_FALSE); const char *_confirm = (*env)->GetStringUTFChars(env, confirm, JNI_FALSE); jlong _ctrl = (jlong) 0; - jstring res = (*env)->NewStringUTF(env, chat_migrate_init(_dbPath, _dbKey, _confirm, &_ctrl)); + jstring res = decode_and_free(env, chat_migrate_init(_dbPath, _dbKey, _confirm, &_ctrl)); (*env)->ReleaseStringUTFChars(env, dbPath, _dbPath); (*env)->ReleaseStringUTFChars(env, dbKey, _dbKey); (*env)->ReleaseStringUTFChars(env, confirm, _confirm); @@ -99,7 +109,7 @@ Java_chat_simplex_common_platform_CoreKt_chatMigrateInit(JNIEnv *env, __unused j JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatCloseStore(JNIEnv *env, __unused jclass clazz, jlong controller) { - jstring res = (*env)->NewStringUTF(env, chat_close_store((void*)controller)); + jstring res = decode_and_free(env, chat_close_store((void*)controller)); return res; } @@ -109,7 +119,7 @@ Java_chat_simplex_common_platform_CoreKt_chatSendCmdRetry(JNIEnv *env, __unused //jint length = (jint) (*env)->GetStringUTFLength(env, msg); //for (int i = 0; i < length; ++i) // __android_log_print(ANDROID_LOG_ERROR, "simplex", "%d: %02x\n", i, _msg[i]); - jstring res = (*env)->NewStringUTF(env, chat_send_cmd_retry((void*)controller, _msg, retryNum)); + jstring res = decode_and_free(env, chat_send_cmd_retry((void*)controller, _msg, retryNum)); (*env)->ReleaseStringUTFChars(env, msg, _msg); return res; } @@ -117,25 +127,25 @@ Java_chat_simplex_common_platform_CoreKt_chatSendCmdRetry(JNIEnv *env, __unused JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatSendRemoteCmdRetry(JNIEnv *env, __unused jclass clazz, jlong controller, jint rhId, jstring msg, jint retryNum) { const char *_msg = (*env)->GetStringUTFChars(env, msg, JNI_FALSE); - jstring res = (*env)->NewStringUTF(env, chat_send_remote_cmd_retry((void*)controller, rhId, _msg, retryNum)); + jstring res = decode_and_free(env, chat_send_remote_cmd_retry((void*)controller, rhId, _msg, retryNum)); (*env)->ReleaseStringUTFChars(env, msg, _msg); return res; } JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatRecvMsg(JNIEnv *env, __unused jclass clazz, jlong controller) { - return (*env)->NewStringUTF(env, chat_recv_msg((void*)controller)); + return decode_and_free(env, chat_recv_msg((void*)controller)); } JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatRecvMsgWait(JNIEnv *env, __unused jclass clazz, jlong controller, jint wait) { - return (*env)->NewStringUTF(env, chat_recv_msg_wait((void*)controller, wait)); + return decode_and_free(env, chat_recv_msg_wait((void*)controller, wait)); } JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatParseMarkdown(JNIEnv *env, __unused jclass clazz, jstring str) { const char *_str = (*env)->GetStringUTFChars(env, str, JNI_FALSE); - jstring res = (*env)->NewStringUTF(env, chat_parse_markdown(_str)); + jstring res = decode_and_free(env, chat_parse_markdown(_str)); (*env)->ReleaseStringUTFChars(env, str, _str); return res; } @@ -143,7 +153,7 @@ Java_chat_simplex_common_platform_CoreKt_chatParseMarkdown(JNIEnv *env, __unused JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatParseServer(JNIEnv *env, __unused jclass clazz, jstring str) { const char *_str = (*env)->GetStringUTFChars(env, str, JNI_FALSE); - jstring res = (*env)->NewStringUTF(env, chat_parse_server(_str)); + jstring res = decode_and_free(env, chat_parse_server(_str)); (*env)->ReleaseStringUTFChars(env, str, _str); return res; } @@ -151,7 +161,7 @@ Java_chat_simplex_common_platform_CoreKt_chatParseServer(JNIEnv *env, __unused j JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatParseUri(JNIEnv *env, __unused jclass clazz, jstring str, jint safe) { const char *_str = (*env)->GetStringUTFChars(env, str, JNI_FALSE); - jstring res = (*env)->NewStringUTF(env, chat_parse_uri(_str, safe)); + jstring res = decode_and_free(env, chat_parse_uri(_str, safe)); (*env)->ReleaseStringUTFChars(env, str, _str); return res; } @@ -160,7 +170,7 @@ JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatPasswordHash(JNIEnv *env, __unused jclass clazz, jstring pwd, jstring salt) { const char *_pwd = (*env)->GetStringUTFChars(env, pwd, JNI_FALSE); const char *_salt = (*env)->GetStringUTFChars(env, salt, JNI_FALSE); - jstring res = (*env)->NewStringUTF(env, chat_password_hash(_pwd, _salt)); + jstring res = decode_and_free(env, chat_password_hash(_pwd, _salt)); (*env)->ReleaseStringUTFChars(env, pwd, _pwd); (*env)->ReleaseStringUTFChars(env, salt, _salt); return res; @@ -169,7 +179,7 @@ Java_chat_simplex_common_platform_CoreKt_chatPasswordHash(JNIEnv *env, __unused JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatValidName(JNIEnv *env, jclass clazz, jstring name) { const char *_name = (*env)->GetStringUTFChars(env, name, JNI_FALSE); - jstring res = (*env)->NewStringUTF(env, chat_valid_name(_name)); + jstring res = decode_and_free(env, chat_valid_name(_name)); (*env)->ReleaseStringUTFChars(env, name, _name); return res; } @@ -177,7 +187,7 @@ Java_chat_simplex_common_platform_CoreKt_chatValidName(JNIEnv *env, jclass clazz JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatParseBadgeCode(JNIEnv *env, jclass clazz, jstring code) { const char *_code = (*env)->GetStringUTFChars(env, code, JNI_FALSE); - jstring res = (*env)->NewStringUTF(env, chat_parse_badge_code(_code)); + jstring res = decode_and_free(env, chat_parse_badge_code(_code)); (*env)->ReleaseStringUTFChars(env, code, _code); return res; } @@ -195,7 +205,7 @@ Java_chat_simplex_common_platform_CoreKt_chatWriteFile(JNIEnv *env, jclass clazz const char *_path = (*env)->GetStringUTFChars(env, path, JNI_FALSE); jbyte *buff = (jbyte *) (*env)->GetDirectBufferAddress(env, buffer); jlong capacity = (*env)->GetDirectBufferCapacity(env, buffer); - jstring res = (*env)->NewStringUTF(env, chat_write_file((void*)controller, _path, buff, capacity)); + jstring res = decode_and_free(env, chat_write_file((void*)controller, _path, buff, capacity)); (*env)->ReleaseStringUTFChars(env, path, _path); return res; } @@ -229,6 +239,7 @@ Java_chat_simplex_common_platform_CoreKt_chatReadFile(JNIEnv *env, jclass clazz, arr = (*env)->NewByteArray(env, len); (*env)->SetByteArrayRegion(env, arr, 0, len, res + 1); } + free(res); jobjectArray ret = (jobjectArray)(*env)->NewObjectArray(env, 2, (*env)->FindClass(env, "java/lang/Object"), NULL); jobject statusObj = (*env)->NewObject(env, (*env)->FindClass(env, "java/lang/Integer"), @@ -243,7 +254,7 @@ JNIEXPORT jstring JNICALL Java_chat_simplex_common_platform_CoreKt_chatEncryptFile(JNIEnv *env, jclass clazz, jlong controller, jstring from_path, jstring to_path) { const char *_from_path = (*env)->GetStringUTFChars(env, from_path, JNI_FALSE); const char *_to_path = (*env)->GetStringUTFChars(env, to_path, JNI_FALSE); - jstring res = (*env)->NewStringUTF(env, chat_encrypt_file((void*)controller, _from_path, _to_path)); + jstring res = decode_and_free(env, chat_encrypt_file((void*)controller, _from_path, _to_path)); (*env)->ReleaseStringUTFChars(env, from_path, _from_path); (*env)->ReleaseStringUTFChars(env, to_path, _to_path); return res; @@ -255,7 +266,7 @@ Java_chat_simplex_common_platform_CoreKt_chatDecryptFile(JNIEnv *env, jclass cla const char *_key = (*env)->GetStringUTFChars(env, key, JNI_FALSE); const char *_nonce = (*env)->GetStringUTFChars(env, nonce, JNI_FALSE); const char *_to_path = (*env)->GetStringUTFChars(env, to_path, JNI_FALSE); - jstring res = (*env)->NewStringUTF(env, chat_decrypt_file(_from_path, _key, _nonce, _to_path)); + jstring res = decode_and_free(env, chat_decrypt_file(_from_path, _key, _nonce, _to_path)); (*env)->ReleaseStringUTFChars(env, from_path, _from_path); (*env)->ReleaseStringUTFChars(env, key, _key); (*env)->ReleaseStringUTFChars(env, nonce, _nonce); diff --git a/apps/multiplatform/common/src/commonMain/cpp/desktop/simplex-api.c b/apps/multiplatform/common/src/commonMain/cpp/desktop/simplex-api.c index 6bb6ffbe97..fddeb16204 100644 --- a/apps/multiplatform/common/src/commonMain/cpp/desktop/simplex-api.c +++ b/apps/multiplatform/common/src/commonMain/cpp/desktop/simplex-api.c @@ -64,6 +64,7 @@ jstring decode_to_utf8_string(JNIEnv *env, char *string) { (*env)->DeleteLocalRef(env, bb); (*env)->DeleteLocalRef(env, charset); (*env)->DeleteLocalRef(env, cb); + free(string); return res; } @@ -239,6 +240,7 @@ Java_chat_simplex_common_platform_CoreKt_chatReadFile(JNIEnv *env, jclass clazz, arr = (*env)->NewByteArray(env, len); (*env)->SetByteArrayRegion(env, arr, 0, len, res + 1); } + free(res); jobjectArray ret = (jobjectArray)(*env)->NewObjectArray(env, 2, (*env)->FindClass(env, "java/lang/Object"), NULL); jobject statusObj = (*env)->NewObject(env, (*env)->FindClass(env, "java/lang/Integer"), diff --git a/plans/2026-09-26-free-core-strings-in-jni.md b/plans/2026-09-26-free-core-strings-in-jni.md new file mode 100644 index 0000000000..effd2374a7 --- /dev/null +++ b/plans/2026-09-26-free-core-strings-in-jni.md @@ -0,0 +1,103 @@ +# Free strings returned by the core in the JNI bindings + +## Problem + +On desktop and Android, resident memory grows for as long as the app runs and is +only released by restarting it. On a profile with ~200k connections it reaches +several GB within a day; the growth continues while the app is idle. + +None of it is visible to either heap tool. The JVM heap stays small, and the +Haskell heap accounts for its own region only — the memory is in the C allocator, +which neither profiler walks. + +## Cause + +Every `chat_*` entry point returns a string that the core allocates with `malloc` +and hands to the caller. `Simplex.Chat.Mobile.Shared` is explicit about it: + +```haskell +newCStringFromLazyBS :: LB.ByteString -> IO CString +newCStringFromLazyBS s = do + ... + buf <- mallocBytes (len + 1) +``` + +The caller owns that buffer. iOS honours the contract — `SimpleXChat/API.swift` +calls `free(c)` after copying each response into a Swift string. The two JNI +bindings never do. + +Both of them copy the bytes into a Java string and then drop the pointer: + +- desktop `decode_to_utf8_string` wraps the buffer in a `DirectByteBuffer`, + decodes it through `Charset.decode`, returns the resulting `String`, and + deletes only the JNI local references; +- Android calls `NewStringUTF(env, chat_*())` inline at every entry point, so the + pointer is never even bound to a variable. + +`chatReadFile` leaks the same way on both platforms, after `SetByteArrayRegion` +has already copied the payload. + +So every command, every received event, and every file read leaks its whole +response. Nothing bounds it: the buffers are unreachable from both runtimes, the +Java string is a copy, and no code path retains the original. + +### Measurements + +A direct-FFI harness calling `chat_send_cmd_retry` in a loop, outside the JVM, +separates the two cases cleanly: + +| harness | behaviour | +|---|---| +| response pointer dropped (current bindings) | +5 KB per call, linear, no plateau | +| response pointer freed | rises to 138 MB and stays flat | + +Per interface event the mean response is ~3.9 KB, so the rate follows activity +rather than connection count. + +On a desktop client running the affected profile, `/proc//smaps` attributes +roughly 1.0 GB to glibc's non-main arenas — sixteen mappings of 63-64 MB, which is +`HEAP_MAX_SIZE` on x86-64, aligned to 64 MB boundaries. Those pages are counted as +touched, not merely reserved. Over the ~10 hours of uptime that is on the order of +100 MB/hour, against a Haskell heap of 1.70 GB and a JVM heap of 328 MB in the +same process. + +## Fix + +Free each buffer once its contents have been copied. + +Desktop already has a single funnel, `decode_to_utf8_string`, which every entry +point goes through, so one `free` covers all of them. Android has no such funnel +because `NewStringUTF` is called inline, so the fix adds `decode_and_free` and +routes the sixteen call sites through it. `chatReadFile` frees its buffer directly +on both platforms. + +Android cannot include `stdlib.h` — the file defines a `reallocarray` stub that +conflicts with it — so `free` is declared on its own. + +The change is confined to the two binding files and does not alter what either +function returns. + +### Why this is safe + +In every case the copy is complete before the buffer is freed: + +- `Charset.decode` produces a `CharBuffer` whose backing array is freshly + allocated, and `toString` copies again into the `String`; +- `NewStringUTF` copies into a new Java string; +- `SetByteArrayRegion` copies into the `byte[]` before either `free` runs. + +Nothing retains the original pointer. On desktop the `DirectByteBuffer` is a view +over it, but it is a JNI local reference deleted in the same function and never +escapes to Java. + +Note that the *input* direction was already correct by accident: the bindings pass +the `malloc`ed buffers from `encode_to_utf8_chars` to `ReleaseStringUTFChars`, +which is not the matching deallocator but reaches `free` through HotSpot's +`FreeHeap`. That mismatch is left alone here. + +## Verification + +- Desktop AppImage with the fix, on a fresh profile: flat at 806-808 MB RSS + through 35 minutes of traffic followed by 10 minutes idle. +- Both `free` calls confirmed in the shipped binary by disassembling + `libapp-lib.so` out of the AppImage.