Free closed peer connections and data channel observers

webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaUtils.h

@@ -115,4 +115,33 @@ void SetHandle(JNIEnv * env, jobject obj, T * t)
 	SetHandle<T>(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<typename T>
+void ReplaceNativeObserver(JNIEnv * env, jobject obj, const std::string & handleName, T * observer)
+{
+	T * old = GetHandle<T>(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<typename T>
+void ClearNativeObserver(JNIEnv * env, jobject obj, const std::string & handleName)
+{
+	T * observer = GetHandle<T>(env, obj, handleName);
+
+	if (observer) {
+		SetHandle<std::nullptr_t>(env, obj, handleName, nullptr);
+		delete observer;
+	}
+}
+
 #endif

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<webrtc::DataChannelInterface>(env, caller);
 	CHECK_HANDLE(channel);
 
-	channel->RegisterObserver(new jni::RTCDataChannelObserver(env, jni::JavaGlobalRef<jobject>(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<jobject>(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<jni::RTCDataChannelObserver>(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<webrtc::DataChannelInterface>(env, caller);
 	CHECK_HANDLE(channel);
 
+	channel->UnregisterObserver();
+
+	ClearNativeObserver<jni::RTCDataChannelObserver>(env, caller, "observerHandle");
+
 	webrtc::RefCountReleaseStatus status = channel->Release();
 
 	if (status != webrtc::RefCountReleaseStatus::kDroppedLastRef) {

webrtc-jni/src/main/cpp/src/JNI_RTCPeerConnection.cpp

@@ -458,12 +458,11 @@ JNIEXPORT void JNICALL Java_dev_kastle_webrtc_RTCPeerConnection_close
 
 		SetHandle<std::nullptr_t>(env, caller, nullptr);
 
-		auto observer = GetHandle<webrtc::PeerConnectionObserver>(env, caller, "observerHandle");
+		ClearNativeObserver<webrtc::PeerConnectionObserver>(env, caller, "observerHandle");
 
-		if (observer) {
-		    SetHandle<std::nullptr_t>(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);

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<jlong>(sent_data_size));
 
 		ExceptionCheck(env);

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.
 	 */