From c857dc0bd946e8edd9c03a35500fcf2ea43fadf3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 01:23:30 +0000 Subject: [PATCH 1/2] Free the local references made for each inbound data channel message OnMessage runs on a native WebRTC thread that stays attached to the JVM, so local references created there are only freed when they are deleted explicitly. Two leaked on every message: the RTCDataChannelBuffer handed to the observer (released from its JavaLocalRef instead of kept by it) and the direct ByteBuffer it wraps. A busy server keeps millions of them alive, growing the heap by roughly 100 MB an hour until it runs out. Keep both in a JavaLocalRef so they are deleted when they go out of scope. The observer only reads the buffer during the call, as the native memory behind it is freed when OnMessage returns anyway. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015jA6r9hShSX7PhQWhVa1g1 --- gradle.properties | 2 +- webrtc-jni/src/main/cpp/src/api/DataBufferFactory.cpp | 6 ++++-- webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp | 5 ++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/gradle.properties b/gradle.properties index 89429d7..ea59365 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1 +1 @@ -version=1.0.4 +version=1.0.4-ip.1 diff --git a/webrtc-jni/src/main/cpp/src/api/DataBufferFactory.cpp b/webrtc-jni/src/main/cpp/src/api/DataBufferFactory.cpp index 619b376..3f6ccc6 100644 --- a/webrtc-jni/src/main/cpp/src/api/DataBufferFactory.cpp +++ b/webrtc-jni/src/main/cpp/src/api/DataBufferFactory.cpp @@ -27,10 +27,12 @@ namespace jni JavaLocalRef DataBufferFactory::create(JNIEnv * env, const webrtc::DataBuffer * dataBuffer) const { - jobject directBuffer = env->NewDirectByteBuffer(const_cast(dataBuffer->data.data()), dataBuffer->data.size()); + // Held by a JavaLocalRef so the local reference is deleted once the buffer object + // holds its own reference. Callers on attached native threads never free it otherwise. + JavaLocalRef directBuffer(env, env->NewDirectByteBuffer(const_cast(dataBuffer->data.data()), dataBuffer->data.size())); const jboolean isBinary = static_cast(dataBuffer->binary); - jobject object = env->NewObject(javaClass, javaCtor, directBuffer, isBinary); + jobject object = env->NewObject(javaClass, javaCtor, directBuffer.get(), isBinary); ExceptionCheck(env); return JavaLocalRef(env, object); diff --git a/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp b/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp index c93c29f..aa20c29 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp @@ -40,9 +40,12 @@ namespace jni { JNIEnv * env = AttachCurrentThread(); + // This runs on a native WebRTC thread that stays attached to the JVM, so a local + // reference is never freed by returning to Java. Keep ownership with jBuffer, so the + // reference is deleted when it goes out of scope instead of leaking on every message. JavaLocalRef jBuffer = bufferFactory->create(env, &buffer); - env->CallVoidMethod(observer, javaClass->onMessage, jBuffer.release()); + env->CallVoidMethod(observer, javaClass->onMessage, jBuffer.get()); ExceptionCheck(env); } From 7601df4d8559eb7771f048b793547f46b27bb493 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 01:30:05 +0000 Subject: [PATCH 2/2] Free closed peer connections and data channel observers Port the leak fixes from devopvoid/webrtc-java@8b77ec8 (#283) that apply to this data-channel-only fork: - RTCPeerConnection.close() cleared the Java handle but never released the reference createPeerConnection() had handed to Java, so every closed connection leaked the native PeerConnection and what it owned. - RTCDataChannel.registerObserver() allocated a native observer and never kept it, so unregisterObserver() could not free it. Each one leaked along with the JNI global reference to the Java observer, which kept the whole closed Java channel reachable. The observer is now stored in an observerHandle field and freed when it is replaced, unregistered, or the channel is disposed. - The data channel observer drops a callback instead of crashing when its thread can no longer attach to the JVM, during shutdown. The ReplaceNativeObserver/ClearNativeObserver helpers come from the same upstream commit. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015jA6r9hShSX7PhQWhVa1g1 --- .../jni-voithos/include/JavaUtils.h | 29 +++++++++++++++++++ .../src/main/cpp/src/JNI_RTCDataChannel.cpp | 15 +++++++++- .../main/cpp/src/JNI_RTCPeerConnection.cpp | 9 +++--- .../cpp/src/api/RTCDataChannelObserver.cpp | 12 ++++++++ .../dev/kastle/webrtc/RTCDataChannel.java | 10 +++++++ 5 files changed, 69 insertions(+), 6 deletions(-) diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaUtils.h b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaUtils.h index 28c1305..575a158 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaUtils.h +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaUtils.h @@ -115,4 +115,33 @@ void SetHandle(JNIEnv * env, jobject obj, T * t) SetHandle(env, obj, "nativeHandle", t); } +// Replaces the native observer stored in the `handleName` field on `obj` +// with `observer`, deleting whatever was stored there before. Use this +// together with ClearNativeObserver() wherever a Java object registers a +// heap-allocated native observer against a "replace the previous one" +// register() API, so the previous observer's lifetime can never be +// forgotten and leaked. +template +void ReplaceNativeObserver(JNIEnv * env, jobject obj, const std::string & handleName, T * observer) +{ + T * old = GetHandle(env, obj, handleName); + + delete old; + + SetHandle(env, obj, handleName, observer); +} + +// Deletes and clears the native observer stored in the `handleName` field +// on `obj`, if any. Safe to call when no observer is currently registered. +template +void ClearNativeObserver(JNIEnv * env, jobject obj, const std::string & handleName) +{ + T * observer = GetHandle(env, obj, handleName); + + if (observer) { + SetHandle(env, obj, handleName, nullptr); + delete observer; + } +} + #endif \ No newline at end of file diff --git a/webrtc-jni/src/main/cpp/src/JNI_RTCDataChannel.cpp b/webrtc-jni/src/main/cpp/src/JNI_RTCDataChannel.cpp index b1300fb..a0ed49f 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_RTCDataChannel.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_RTCDataChannel.cpp @@ -30,7 +30,14 @@ JNIEXPORT void JNICALL Java_dev_kastle_webrtc_RTCDataChannel_registerObserver webrtc::DataChannelInterface * channel = GetHandle(env, caller); CHECK_HANDLE(channel); - channel->RegisterObserver(new jni::RTCDataChannelObserver(env, jni::JavaGlobalRef(env, jObserver))); + // Unregister and free a previously registered observer before replacing + // it, otherwise it would leak along with its JNI global reference. + channel->UnregisterObserver(); + + auto observer = new jni::RTCDataChannelObserver(env, jni::JavaGlobalRef(env, jObserver)); + ReplaceNativeObserver(env, caller, "observerHandle", observer); + + channel->RegisterObserver(observer); } JNIEXPORT void JNICALL Java_dev_kastle_webrtc_RTCDataChannel_unregisterObserver @@ -40,6 +47,8 @@ JNIEXPORT void JNICALL Java_dev_kastle_webrtc_RTCDataChannel_unregisterObserver CHECK_HANDLE(channel); channel->UnregisterObserver(); + + ClearNativeObserver(env, caller, "observerHandle"); } JNIEXPORT jstring JNICALL Java_dev_kastle_webrtc_RTCDataChannel_getLabel @@ -147,6 +156,10 @@ JNIEXPORT void JNICALL Java_dev_kastle_webrtc_RTCDataChannel_dispose webrtc::DataChannelInterface * channel = GetHandle(env, caller); CHECK_HANDLE(channel); + channel->UnregisterObserver(); + + ClearNativeObserver(env, caller, "observerHandle"); + webrtc::RefCountReleaseStatus status = channel->Release(); if (status != webrtc::RefCountReleaseStatus::kDroppedLastRef) { diff --git a/webrtc-jni/src/main/cpp/src/JNI_RTCPeerConnection.cpp b/webrtc-jni/src/main/cpp/src/JNI_RTCPeerConnection.cpp index f388eec..70e0e94 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_RTCPeerConnection.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_RTCPeerConnection.cpp @@ -458,12 +458,11 @@ JNIEXPORT void JNICALL Java_dev_kastle_webrtc_RTCPeerConnection_close SetHandle(env, caller, nullptr); - auto observer = GetHandle(env, caller, "observerHandle"); + ClearNativeObserver(env, caller, "observerHandle"); - if (observer) { - SetHandle(env, caller, "observerHandle", nullptr); - delete observer; - } + // Drop the owning reference taken when the PeerConnection was handed + // to Java in PeerConnectionFactory::createPeerConnection. + pc->Release(); } catch (...) { ThrowCxxJavaException(env); diff --git a/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp b/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp index aa20c29..a6d6747 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp @@ -31,6 +31,10 @@ namespace jni { JNIEnv * env = AttachCurrentThread(); + if (env == nullptr) { + return; + } + env->CallVoidMethod(observer, javaClass->onStateChange); ExceptionCheck(env); @@ -40,6 +44,10 @@ namespace jni { JNIEnv * env = AttachCurrentThread(); + if (env == nullptr) { + return; + } + // This runs on a native WebRTC thread that stays attached to the JVM, so a local // reference is never freed by returning to Java. Keep ownership with jBuffer, so the // reference is deleted when it goes out of scope instead of leaking on every message. @@ -54,6 +62,10 @@ namespace jni { JNIEnv * env = AttachCurrentThread(); + if (env == nullptr) { + return; + } + env->CallVoidMethod(observer, javaClass->onBufferedAmountChange, static_cast(sent_data_size)); ExceptionCheck(env); diff --git a/webrtc/src/main/java/dev/kastle/webrtc/RTCDataChannel.java b/webrtc/src/main/java/dev/kastle/webrtc/RTCDataChannel.java index 3da6414..e81f56f 100644 --- a/webrtc/src/main/java/dev/kastle/webrtc/RTCDataChannel.java +++ b/webrtc/src/main/java/dev/kastle/webrtc/RTCDataChannel.java @@ -28,6 +28,16 @@ */ public class RTCDataChannel extends DisposableNativeObject { + /** + * The native observer registered via {@link #registerObserver}. The + * RTCDataChannel does not take ownership of the Java observer, but it + * owns the native observer wrapper; it is freed when replaced, when + * {@link #unregisterObserver()} is called, and when this channel is + * disposed. + */ + @SuppressWarnings("unused") + private long observerHandle; + /** * Used by the native api. */