Bug 973522 MediaRecorder causes large leak. r=roc, jsmith

content/media/MediaRecorder.cpp

@@ -22,12 +22,19 @@
 #include "mozilla/dom/AudioStreamTrack.h"
 #include "mozilla/dom/VideoStreamTrack.h"
 
+#ifdef PR_LOGGING
+PRLogModuleInfo* gMediaRecorderLog;
+#define LOG(type, msg) PR_LOG(gMediaRecorderLog, type, msg)
+#else
+#define LOG(type, msg)
+#endif
+
 namespace mozilla {
 
 namespace dom {
 
-NS_IMPL_CYCLE_COLLECTION_INHERITED_2(MediaRecorder, nsDOMEventTargetHelper,
+NS_IMPL_CYCLE_COLLECTION_INHERITED_1(MediaRecorder, nsDOMEventTargetHelper,
-                                     mStream, mSession)
+                                     mStream)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(MediaRecorder)
 NS_INTERFACE_MAP_END_INHERITING(nsDOMEventTargetHelper)
@@ -81,9 +88,14 @@ class MediaRecorder::Session: public nsIObserver
 
     NS_IMETHODIMP Run()
     {
+      LOG(PR_LOG_DEBUG, ("Session.PushBlobRunnable s=(%p)", mSession.get()));
       MOZ_ASSERT(NS_IsMainThread());
 
-      MediaRecorder *recorder = mSession->mRecorder;
+      nsRefPtr<MediaRecorder> recorder = mSession->mRecorder;
+      if (!recorder) {
+	 return NS_OK;
+      }
+      recorder->SetMimeType(mSession->mMimeType);
       if (mSession->IsEncoderError()) {
         recorder->NotifyError(NS_ERROR_UNEXPECTED);
       }
@@ -91,6 +103,7 @@ class MediaRecorder::Session: public nsIObserver
       if (NS_FAILED(rv)) {
         recorder->NotifyError(rv);
       }
+
       return NS_OK;
     }
 
@@ -111,6 +124,7 @@ class MediaRecorder::Session: public nsIObserver
       MOZ_ASSERT(NS_GetCurrentThread() == mSession->mReadThread);
 
       mSession->Extract();
+      LOG(PR_LOG_DEBUG, ("Session.ExtractRunnable shutdown = %d", mSession->mEncoder->IsShutdown()));
       if (!mSession->mEncoder->IsShutdown()) {
         NS_DispatchToCurrentThread(new ExtractRunnable(mSession));
       } else {
@@ -123,7 +137,7 @@ class MediaRecorder::Session: public nsIObserver
     }
 
   private:
-    Session* mSession;
+    nsRefPtr<Session> mSession;
   };
 
   // For Ensure recorder has tracks to record.
@@ -149,12 +163,12 @@ class MediaRecorder::Session: public nsIObserver
           trackType |= DOMMediaStream::HINT_CONTENTS_AUDIO;
         }
       }
+      LOG(PR_LOG_DEBUG, ("Session.NotifyTracksAvailable track type = (%d)", trackType));
       mSession->AfterTracksAdded(trackType);
     }
   private:
     nsRefPtr<Session> mSession;
   };
-
   // Main thread task.
   // To delete RecordingSession object.
   class DestroyRunnable : public nsRunnable
@@ -165,9 +179,13 @@ class MediaRecorder::Session: public nsIObserver
 
     NS_IMETHODIMP Run()
     {
+      LOG(PR_LOG_DEBUG, ("Session.DestroyRunnable session refcnt = (%d) stopIssued %d s=(%p)",
+                         (int)mSession->mRefCnt, mSession->mStopIssued, mSession.get()));
       MOZ_ASSERT(NS_IsMainThread() && mSession.get());
-      MediaRecorder *recorder = mSession->mRecorder;
+      nsRefPtr<MediaRecorder> recorder = mSession->mRecorder;
-
+      if (!recorder) {
+        return NS_OK;
+      }
       // SourceMediaStream is ended, and send out TRACK_EVENT_END notification.
       // Read Thread will be terminate soon.
       // We need to switch MediaRecorder to "Stop" state first to make sure
@@ -176,16 +194,18 @@ class MediaRecorder::Session: public nsIObserver
       // Also avoid to run if this session already call stop before
       if (!mSession->mStopIssued) {
         ErrorResult result;
+        mSession->mStopIssued = true;
         recorder->Stop(result);
         NS_DispatchToMainThread(new DestroyRunnable(mSession.forget()));
-
         return NS_OK;
       }
 
       // Dispatch stop event and clear MIME type.
-      recorder->DispatchSimpleEvent(NS_LITERAL_STRING("stop"));
       mSession->mMimeType = NS_LITERAL_STRING("");
       recorder->SetMimeType(mSession->mMimeType);
+      recorder->DispatchSimpleEvent(NS_LITERAL_STRING("stop"));
+      recorder->RemoveSession(mSession);
+      mSession->mRecorder = nullptr;
       return NS_OK;
     }
 
@@ -215,11 +235,13 @@ public:
   // Only DestroyRunnable is allowed to delete Session object.
   virtual ~Session()
   {
+    LOG(PR_LOG_DEBUG, ("Session.~Session (%p)", this));
     CleanupStreams();
   }
 
   void Start()
   {
+    LOG(PR_LOG_DEBUG, ("Session.Start %p", this));
     MOZ_ASSERT(NS_IsMainThread());
 
     SetupStreams();
@@ -227,8 +249,8 @@ public:
 
   void Stop()
   {
+    LOG(PR_LOG_DEBUG, ("Session.Stop %p", this));
     MOZ_ASSERT(NS_IsMainThread());
-
     mStopIssued = true;
     CleanupStreams();
     nsContentUtils::UnregisterShutdownObserver(this);
@@ -236,6 +258,7 @@ public:
 
   nsresult Pause()
   {
+    LOG(PR_LOG_DEBUG, ("Session.Pause"));
     MOZ_ASSERT(NS_IsMainThread());
 
     NS_ENSURE_TRUE(mTrackUnionStream, NS_ERROR_FAILURE);
@@ -246,6 +269,7 @@ public:
 
   nsresult Resume()
   {
+    LOG(PR_LOG_DEBUG, ("Session.Resume"));
     MOZ_ASSERT(NS_IsMainThread());
 
     NS_ENSURE_TRUE(mTrackUnionStream, NS_ERROR_FAILURE);
@@ -256,6 +280,7 @@ public:
 
   already_AddRefed<nsIDOMBlob> GetEncodedData()
   {
+    MOZ_ASSERT(NS_IsMainThread());
     return mEncodedBufferCache->ExtractBlob(mMimeType);
   }
 
@@ -266,6 +291,11 @@ public:
     }
     return false;
   }
+  void ForgetMediaRecorder()
+  {
+    LOG(PR_LOG_DEBUG, ("Session.ForgetMediaRecorder (%p)", mRecorder));
+    mRecorder = nullptr;
+  }
 private:
 
   // Pull encoded meida data from MediaEncoder and put into EncodedBufferCache.
@@ -273,14 +303,13 @@ private:
   void Extract()
   {
     MOZ_ASSERT(NS_GetCurrentThread() == mReadThread);
-
+    LOG(PR_LOG_DEBUG, ("Session.Extract %p", this));
     // Whether push encoded data back to onDataAvailable automatically.
     const bool pushBlob = (mTimeSlice > 0) ? true : false;
 
     // Pull encoded media data from MediaEncoder
     nsTArray<nsTArray<uint8_t> > encodedBuf;
     mEncoder->GetEncodedData(&encodedBuf, mMimeType);
-    mRecorder->SetMimeType(mMimeType);
 
     // Append pulled data into cache buffer.
     for (uint32_t i = 0; i < encodedBuf.Length(); i++) {
@@ -311,12 +340,13 @@ private:
     mInputPort = mTrackUnionStream->AllocateInputPort(mRecorder->mStream->GetStream(), MediaInputPort::FLAG_BLOCK_OUTPUT);
 
     // Allocate encoder and bind with the Track Union Stream.
-    TracksAvailableCallback* tracksAvailableCallback = new TracksAvailableCallback(mRecorder->mSession);
+    TracksAvailableCallback* tracksAvailableCallback = new TracksAvailableCallback(mRecorder->mSessions.LastElement());
     mRecorder->mStream->OnTracksAvailable(tracksAvailableCallback);
   }
 
   void AfterTracksAdded(uint8_t aTrackTypes)
   {
+    LOG(PR_LOG_DEBUG, ("Session.AfterTracksAdded %p", this));
     MOZ_ASSERT(NS_IsMainThread());
 
     // Allocate encoder and bind with union stream.
@@ -367,10 +397,10 @@ private:
   void DoSessionEndTask(nsresult rv)
   {
     MOZ_ASSERT(NS_IsMainThread());
-
     if (NS_FAILED(rv)) {
       mRecorder->NotifyError(rv);
     }
+
     CleanupStreams();
     // Destroy this session object in main thread.
     NS_DispatchToMainThread(new PushBlobRunnable(this));
@@ -392,9 +422,14 @@ private:
   NS_IMETHODIMP Observe(nsISupports *aSubject, const char *aTopic, const char16_t *aData)
   {
     MOZ_ASSERT(NS_IsMainThread());
-
+    LOG(PR_LOG_DEBUG, ("Session.Observe XPCOM_SHUTDOWN %p", this));
     if (strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {
       // Force stop Session to terminate Read Thread.
+      mEncoder->Cancel();
+      if (mRecorder) {
+        mRecorder->RemoveSession(this);
+        mRecorder = nullptr;
+      }
       Stop();
     }
 
@@ -402,9 +437,8 @@ private:
   }
 
 private:
-  // Hold a reference to MediaRecoder to make sure MediaRecoder be
+  // Hold weak a reference to MediaRecoder and can be accessed ONLY on main thread.
-  // destroyed after all session object dead.
+  MediaRecorder* mRecorder;
-  nsRefPtr<MediaRecorder> mRecorder;
 
   // Receive track data from source and dispatch to Encoder.
   // Pause/ Resume controller.
@@ -434,18 +468,28 @@ NS_IMPL_ISUPPORTS1(MediaRecorder::Session, nsIObserver)
 
 MediaRecorder::~MediaRecorder()
 {
-  MOZ_ASSERT(mSession == nullptr);
+  LOG(PR_LOG_DEBUG, ("~MediaRecorder (%p)", this));
+  for (uint32_t i = 0; i < mSessions.Length(); i ++) {
+    if (mSessions[i]) {
+      mSessions[i]->ForgetMediaRecorder();
+      mSessions[i]->Stop();
+    }
+  }
 }
 
 MediaRecorder::MediaRecorder(DOMMediaStream& aStream, nsPIDOMWindow* aOwnerWindow)
   : nsDOMEventTargetHelper(aOwnerWindow),
     mState(RecordingState::Inactive),
-    mSession(nullptr),
     mMutex("Session.Data.Mutex")
 {
   MOZ_ASSERT(aOwnerWindow);
   MOZ_ASSERT(aOwnerWindow->IsInnerWindow());
   mStream = &aStream;
+#ifdef PR_LOGGING
+  if (!gMediaRecorderLog) {
+    gMediaRecorderLog = PR_NewLogModule("MediaRecorder");
+  }
+#endif
 }
 
 void
@@ -465,6 +509,7 @@ MediaRecorder::GetMimeType(nsString &aMimeType)
 void
 MediaRecorder::Start(const Optional<int32_t>& aTimeSlice, ErrorResult& aResult)
 {
+  LOG(PR_LOG_DEBUG, ("MediaRecorder.Start %p", this));
   if (mState != RecordingState::Inactive) {
     aResult.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
@@ -497,59 +542,60 @@ MediaRecorder::Start(const Optional<int32_t>& aTimeSlice, ErrorResult& aResult)
 
   mState = RecordingState::Recording;
   // Start a session
-  mSession = new Session(this, timeSlice);
+
-  mSession->Start();
+  mSessions.AppendElement();
+  mSessions.LastElement() = new Session(this, timeSlice);
+  mSessions.LastElement()->Start();
 }
 
 void
 MediaRecorder::Stop(ErrorResult& aResult)
 {
+  LOG(PR_LOG_DEBUG, ("MediaRecorder.Stop %p", this));
   if (mState == RecordingState::Inactive) {
     aResult.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
   mState = RecordingState::Inactive;
-
+  if (mSessions.Length() > 0) {
-  mSession->Stop();
+    mSessions.LastElement()->Stop();
-  mSession = nullptr;
+  }
 }
 
 void
 MediaRecorder::Pause(ErrorResult& aResult)
 {
+  LOG(PR_LOG_DEBUG, ("MediaRecorder.Pause"));
   if (mState != RecordingState::Recording) {
     aResult.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
-  MOZ_ASSERT(mSession != nullptr);
+  MOZ_ASSERT(mSessions.Length() > 0);
-  if (mSession) {
+  nsresult rv = mSessions.LastElement()->Pause();
-    nsresult rv = mSession->Pause();
+  if (NS_FAILED(rv)) {
-    if (NS_FAILED(rv)) {
+    NotifyError(rv);
-      NotifyError(rv);
+    return;
-      return;
-    }
-    mState = RecordingState::Paused;
   }
+  mState = RecordingState::Paused;
 }
 
 void
 MediaRecorder::Resume(ErrorResult& aResult)
 {
+  LOG(PR_LOG_DEBUG, ("MediaRecorder.Resume"));
   if (mState != RecordingState::Paused) {
     aResult.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
-  MOZ_ASSERT(mSession != nullptr);
+  MOZ_ASSERT(mSessions.Length() > 0);
-  if (mSession) {
+  nsresult rv = mSessions.LastElement()->Resume();
-    nsresult rv = mSession->Resume();
+  if (NS_FAILED(rv)) {
-    if (NS_FAILED(rv)) {
+    NotifyError(rv);
-      NotifyError(rv);
+    return;
-      return;
-    }
-    mState = RecordingState::Recording;
   }
+  mState = RecordingState::Recording;
 }
 
 class CreateAndDispatchBlobEventRunnable : public nsRunnable {
@@ -580,8 +626,9 @@ MediaRecorder::RequestData(ErrorResult& aResult)
   }
 
   NS_DispatchToMainThread(
-    new CreateAndDispatchBlobEventRunnable(mSession->GetEncodedData(), this),
+    new CreateAndDispatchBlobEventRunnable(mSessions.LastElement()->GetEncodedData(),
-    NS_DISPATCH_NORMAL);
+                                           this),
+                                           NS_DISPATCH_NORMAL);
 }
 
 JSObject*
@@ -617,12 +664,11 @@ nsresult
 MediaRecorder::CreateAndDispatchBlobEvent(already_AddRefed<nsIDOMBlob>&& aBlob)
 {
   NS_ABORT_IF_FALSE(NS_IsMainThread(), "Not running on main thread");
-
   if (!CheckPrincipal()) {
     // Media is not same-origin, don't allow the data out.
+    nsRefPtr<nsIDOMBlob> blob = aBlob;
     return NS_ERROR_DOM_SECURITY_ERR;
   }
-
   BlobEventInit init;
   init.mBubbles = false;
   init.mCancelable = false;
@@ -704,6 +750,10 @@ MediaRecorder::NotifyError(nsresult aRv)
 
 bool MediaRecorder::CheckPrincipal()
 {
+  NS_ABORT_IF_FALSE(NS_IsMainThread(), "Not running on main thread");
+  if (!mStream) {
+    return false;
+  }
   nsCOMPtr<nsIPrincipal> principal = mStream->GetPrincipal();
   if (!GetOwner())
     return false;
@@ -718,5 +768,12 @@ bool MediaRecorder::CheckPrincipal()
   return subsumes;
 }
 
+void
+MediaRecorder::RemoveSession(Session* aSession)
+{
+  LOG(PR_LOG_DEBUG, ("MediaRecorder.RemoveSession (%p)", aSession));
+  mSessions.RemoveElement(aSession);
+}
+
 }
 }

content/media/MediaRecorder.h

@@ -100,13 +100,14 @@ protected:
   void SetMimeType(const nsString &aMimeType);
 
   MediaRecorder(const MediaRecorder& x) MOZ_DELETE; // prevent bad usage
-
+  // Remove session pointer.
+  void RemoveSession(Session* aSession);
   // MediaStream passed from js context
   nsRefPtr<DOMMediaStream> mStream;
   // The current state of the MediaRecorder object.
   RecordingState mState;
-  // Current recording session.
+  // Hold the sessions pointer in media recorder and clean in the destructor of recorder.
-  nsRefPtr<Session> mSession;
+  nsTArray<Session*> mSessions;
   // Thread safe for mMimeType.
   Mutex mMutex;
   // It specifies the container format as well as the audio and video capture formats.

content/media/test/mochitest.ini

@@ -403,6 +403,7 @@ skip-if = (buildapp == 'b2g' && (toolkit != 'gonk' || debug))
 [test_mediarecorder_avoid_recursion.html]
 [test_mediarecorder_record_timeslice.html]
 [test_mediarecorder_record_audiocontext.html]
+[test_mediarecorder_record_audiocontext_mlk.html]
 [test_mediarecorder_record_4ch_audiocontext.html]
 skip-if = (toolkit == 'gonk' && !debug)
 [test_mediarecorder_record_stopms.html]

content/media/test/test_mediarecorder_record_audiocontext_mlk.html

@@ -0,0 +1,24 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>capture for possible memory leak when record AudioContext</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=973765">Mozill
+a Bug 973765</a>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+   // This test case want to capture the memory leak if exit the browser after running those script.
+   var ac = new window.AudioContext();
+   var destStream = ac.createMediaStreamDestination().stream;
+   var recorder = new MediaRecorder(destStream);
+   recorder.start(1000);
+   is(recorder.state, 'recording', 'Media recorder should be recording');
+   is(recorder.stream, destStream,
+      'Media recorder stream = element stream at the start of recording');
+</script>
+</pre>
+</body>
+</html>