Android: Fix memory leak for remote MediaStream

talk/app/webrtc/java/jni/peerconnection_jni.cc

 /*
  * libjingle
  * Copyright 2013 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 188 matching lines @@
         j_audio_track_ctor_(GetMethodID(
             jni, *j_audio_track_class_, "<init>", "(J)V")),
         j_video_track_class_(jni, FindClass(jni, "org/webrtc/VideoTrack")),
         j_video_track_ctor_(GetMethodID(
             jni, *j_video_track_class_, "<init>", "(J)V")),
         j_data_channel_class_(jni, FindClass(jni, "org/webrtc/DataChannel")),
         j_data_channel_ctor_(GetMethodID(
             jni, *j_data_channel_class_, "<init>", "(J)V")) {
   }
 
-  virtual ~PCOJava() {}
+  virtual ~PCOJava() {
+    ScopedLocalRefFrame local_ref_frame(jni());
+    while (!remote_streams_.empty())
+      DisposeRemoteStream(remote_streams_.begin());
+  }
 
   void OnIceCandidate(const IceCandidateInterface* candidate) override {
     ScopedLocalRefFrame local_ref_frame(jni());
     std::string sdp;
     CHECK(candidate->ToString(&sdp)) << "got so far: " << sdp;
     jclass candidate_class = FindClass(jni(), "org/webrtc/IceCandidate");
     jmethodID ctor = GetMethodID(jni(), candidate_class,
         "<init>", "(Ljava/lang/String;ILjava/lang/String;)V");
     jstring j_mid = JavaStringFromStdString(jni(), candidate->sdp_mid());
     jstring j_sdp = JavaStringFromStdString(jni(), sdp);
@@ skipping 45 matching lines @@
         jni(), *j_observer_class_, "onIceGatheringChange",
         "(Lorg/webrtc/PeerConnection$IceGatheringState;)V");
     jobject new_state_enum = JavaEnumFromIndex(
         jni(), "PeerConnection$IceGatheringState", new_state);
     jni()->CallVoidMethod(*j_observer_global_, m, new_state_enum);
     CHECK_EXCEPTION(jni()) << "error during CallVoidMethod";
   }
 
   void OnAddStream(MediaStreamInterface* stream) override {
     ScopedLocalRefFrame local_ref_frame(jni());
-    jobject j_stream = jni()->NewObject(
-        *j_media_stream_class_, j_media_stream_ctor_, (jlong)stream);
+    // Java MediaStream holds one reference. Corresponding Release() is in
+    // MediaStream_free, triggered by MediaStream.dispose().
+    stream->AddRef();
+    jobject j_stream =
+        jni()->NewObject(*j_media_stream_class_, j_media_stream_ctor_,
+                         reinterpret_cast<jlong>(stream));
     CHECK_EXCEPTION(jni()) << "error during NewObject";
 
-    AudioTrackVector audio_tracks = stream->GetAudioTracks();
-    for (size_t i = 0; i < audio_tracks.size(); ++i) {
-      AudioTrackInterface* track = audio_tracks[i];
+    for (const auto& track : stream->GetAudioTracks()) {
       jstring id = JavaStringFromStdString(jni(), track->id());
-      jobject j_track = jni()->NewObject(
-          *j_audio_track_class_, j_audio_track_ctor_, (jlong)track, id);
+      // Java AudioTrack holds one reference. Corresponding Release() is in
+      // MediaStreamTrack_free, triggered by AudioTrack.dispose().
+      track->AddRef();
+      jobject j_track =
+          jni()->NewObject(*j_audio_track_class_, j_audio_track_ctor_,
+                           reinterpret_cast<jlong>(track.get()), id);
       CHECK_EXCEPTION(jni()) << "error during NewObject";
       jfieldID audio_tracks_id = GetFieldID(jni(),
                                             *j_media_stream_class_,
                                             "audioTracks",
                                             "Ljava/util/LinkedList;");
       jobject audio_tracks = GetObjectField(jni(), j_stream, audio_tracks_id);
       jmethodID add = GetMethodID(jni(),
                                   GetObjectClass(jni(), audio_tracks),
                                   "add",
                                   "(Ljava/lang/Object;)Z");
       jboolean added = jni()->CallBooleanMethod(audio_tracks, add, j_track);
       CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod";
       CHECK(added);
     }
 
-    VideoTrackVector video_tracks = stream->GetVideoTracks();
-    for (size_t i = 0; i < video_tracks.size(); ++i) {
-      VideoTrackInterface* track = video_tracks[i];
+    for (const auto& track : stream->GetVideoTracks()) {
       jstring id = JavaStringFromStdString(jni(), track->id());
-      jobject j_track = jni()->NewObject(
-          *j_video_track_class_, j_video_track_ctor_, (jlong)track, id);
+      // Java VideoTrack holds one reference. Corresponding Release() is in
+      // MediaStreamTrack_free, triggered by VideoTrack.dispose().
+      track->AddRef();
+      jobject j_track =
+          jni()->NewObject(*j_video_track_class_, j_video_track_ctor_,
+                           reinterpret_cast<jlong>(track.get()), id);
       CHECK_EXCEPTION(jni()) << "error during NewObject";
       jfieldID video_tracks_id = GetFieldID(jni(),
                                             *j_media_stream_class_,
                                             "videoTracks",
                                             "Ljava/util/LinkedList;");
       jobject video_tracks = GetObjectField(jni(), j_stream, video_tracks_id);
       jmethodID add = GetMethodID(jni(),
                                   GetObjectClass(jni(), video_tracks),
                                   "add",
                                   "(Ljava/lang/Object;)Z");
       jboolean added = jni()->CallBooleanMethod(video_tracks, add, j_track);
       CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod";
       CHECK(added);
     }
-    streams_[stream] = jni()->NewWeakGlobalRef(j_stream);
+    remote_streams_[stream] = jni()->NewGlobalRef(j_stream);

[AlexG, 2015/08/26 17:18:07]

nit: use NewGlobalRef(jni, j_stream) and remove or rename CHECK_EXCEPTION below
- it prints no longer used "NewWeakGlobalRef".

     CHECK_EXCEPTION(jni()) << "error during NewWeakGlobalRef";
 
     jmethodID m = GetMethodID(jni(), *j_observer_class_, "onAddStream",
                               "(Lorg/webrtc/MediaStream;)V");
     jni()->CallVoidMethod(*j_observer_global_, m, j_stream);
     CHECK_EXCEPTION(jni()) << "error during CallVoidMethod";
   }
 
   void OnRemoveStream(MediaStreamInterface* stream) override {
     ScopedLocalRefFrame local_ref_frame(jni());
-    NativeToJavaStreamsMap::iterator it = streams_.find(stream);
-    CHECK(it != streams_.end()) << "unexpected stream: " << std::hex << stream;
-
-    WeakRef s(jni(), it->second);
-    streams_.erase(it);
-    if (!s.obj())
-      return;
-
+    NativeToJavaStreamsMap::iterator it = remote_streams_.find(stream);
+    CHECK(it != remote_streams_.end()) << "unexpected stream: " << std::hex
+                                       << stream;
+    jobject j_stream = it->second;
     jmethodID m = GetMethodID(jni(), *j_observer_class_, "onRemoveStream",
                               "(Lorg/webrtc/MediaStream;)V");
-    jni()->CallVoidMethod(*j_observer_global_, m, s.obj());
+    jni()->CallVoidMethod(*j_observer_global_, m, j_stream);
     CHECK_EXCEPTION(jni()) << "error during CallVoidMethod";
+    DisposeRemoteStream(it);
   }
 
   void OnDataChannel(DataChannelInterface* channel) override {
     ScopedLocalRefFrame local_ref_frame(jni());
     jobject j_channel = jni()->NewObject(
         *j_data_channel_class_, j_data_channel_ctor_, (jlong)channel);
     CHECK_EXCEPTION(jni()) << "error during NewObject";
 
     jmethodID m = GetMethodID(jni(), *j_observer_class_, "onDataChannel",
                               "(Lorg/webrtc/DataChannel;)V");
@@ skipping 18 matching lines @@
   }
 
   void SetConstraints(ConstraintsWrapper* constraints) {
     CHECK(!constraints_.get()) << "constraints already set!";
     constraints_.reset(constraints);
   }
 
   const ConstraintsWrapper* constraints() { return constraints_.get(); }
 
  private:
+  typedef std::map<MediaStreamInterface*, jobject> NativeToJavaStreamsMap;
+
+  void DisposeRemoteStream(const NativeToJavaStreamsMap::iterator& it) {
+    jobject j_stream = it->second;
+    remote_streams_.erase(it);
+    jni()->CallVoidMethod(
+        j_stream, GetMethodID(jni(), *j_media_stream_class_, "dispose", "()V"));
+    CHECK_EXCEPTION(jni()) << "error during MediaStream.dispose()";
+    DeleteGlobalRef(jni(), j_stream);
+  }
+
   JNIEnv* jni() {
     return AttachCurrentThreadIfNeeded();
   }
 
   const ScopedGlobalRef<jobject> j_observer_global_;
   const ScopedGlobalRef<jclass> j_observer_class_;
   const ScopedGlobalRef<jclass> j_media_stream_class_;
   const jmethodID j_media_stream_ctor_;
   const ScopedGlobalRef<jclass> j_audio_track_class_;
   const jmethodID j_audio_track_ctor_;
   const ScopedGlobalRef<jclass> j_video_track_class_;
   const jmethodID j_video_track_ctor_;
   const ScopedGlobalRef<jclass> j_data_channel_class_;
   const jmethodID j_data_channel_ctor_;
-  typedef std::map<void*, jweak> NativeToJavaStreamsMap;
-  NativeToJavaStreamsMap streams_;  // C++ -> Java streams.
+  // C++ -> Java remote streams. The stored jobects are global refs and must be
+  // manually deleted upon removal. Use DisposeRemoteStream().
+  NativeToJavaStreamsMap remote_streams_;
   scoped_ptr<ConstraintsWrapper> constraints_;
 };
 
 // Wrapper for a Java MediaConstraints object.  Copies all needed data so when
 // the constructor returns the Java object is no longer needed.
 class ConstraintsWrapper : public MediaConstraintsInterface {
  public:
   ConstraintsWrapper(JNIEnv* jni, jobject j_constraints) {
     PopulateConstraintsFromJavaPairList(
         jni, j_constraints, "mandatory", &mandatory_);
@@ skipping 1270 matching lines @@
 }
 
 JOW(void, VideoTrack_nativeRemoveRenderer)(
     JNIEnv* jni, jclass,
     jlong j_video_track_pointer, jlong j_renderer_pointer) {
   reinterpret_cast<VideoTrackInterface*>(j_video_track_pointer)->RemoveRenderer(
       reinterpret_cast<VideoRendererInterface*>(j_renderer_pointer));
 }
 
 }  // namespace webrtc_jni