mirror of
https://github.com/simplex-chat/simplex-chat.git
synced 2026-09-27 15:48:54 +00:00
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.
This commit is contained in:
@@ -4,6 +4,9 @@
|
||||
//#include <stdlib.h>
|
||||
//#include <android/log.h>
|
||||
|
||||
// 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);
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -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/<pid>/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.
|
||||
Reference in New Issue
Block a user