From 6d30631f01909fca3ec0415b043a26c19139944c Mon Sep 17 00:00:00 2001
From: Qiang Chen <qiangchen@chromium.org>
Date: Thu, 1 Mar 2018 13:41:44 -0800
Subject: [PATCH] Bug Fix: Multiplex Codec Crash

This CL adds a lock to stashed_images_ in MultiplexEncoderAdapter.
Without lock, it is possible that different threads acts on
stashed_images_ simultaneously and leads to crash.

Bug: webrtc:8965
Change-Id: I887861092d185c3bd6047eb529d8c1cf57fa4648
Reviewed-on: https://webrtc-review.googlesource.com/59260
Reviewed-by: Emircan Uysaler <emircan@webrtc.org>
Commit-Queue: Qiang Chen <qiangchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#22261}
---
 .../include/multiplex_encoder_adapter.h       |  6 ++++-
 .../multiplex/multiplex_encoder_adapter.cc    | 26 ++++++++++++-------
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h b/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h
index 7488f206da..9ba543f909 100644
--- a/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h
+++ b/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h
@@ -20,6 +20,7 @@
 #include "api/video_codecs/video_encoder_factory.h"
 #include "modules/video_coding/codecs/multiplex/include/multiplex_encoded_image_packer.h"
 #include "modules/video_coding/include/video_codec_interface.h"
+#include "rtc_base/criticalsection.h"
 
 namespace webrtc {
 
@@ -66,13 +67,16 @@ class MultiplexEncoderAdapter : public VideoEncoder {
   std::vector<std::unique_ptr<AdapterEncodedImageCallback>> adapter_callbacks_;
   EncodedImageCallback* encoded_complete_callback_;
 
-  std::map<uint32_t /* timestamp */, MultiplexImage> stashed_images_;
+  std::map<uint32_t /* timestamp */, MultiplexImage> stashed_images_
+      RTC_GUARDED_BY(crit_);
 
   uint16_t picture_index_ = 0;
   std::vector<uint8_t> multiplex_dummy_planes_;
 
   int key_frame_interval_;
   EncodedImage combined_image_;
+
+  rtc::CriticalSection crit_;
 };
 
 }  // namespace webrtc
diff --git a/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc b/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc
index 0d797ecedd..7eb0d50f36 100644
--- a/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc
+++ b/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc
@@ -122,10 +122,14 @@ int MultiplexEncoderAdapter::Encode(
   }
   const bool has_alpha = input_image.video_frame_buffer()->type() ==
                          VideoFrameBuffer::Type::kI420A;
-  stashed_images_.emplace(
-      std::piecewise_construct, std::forward_as_tuple(input_image.timestamp()),
-      std::forward_as_tuple(picture_index_,
-                            has_alpha ? kAlphaCodecStreams : 1));
+  {
+    rtc::CritScope cs(&crit_);
+    stashed_images_.emplace(
+        std::piecewise_construct,
+        std::forward_as_tuple(input_image.timestamp()),
+        std::forward_as_tuple(picture_index_,
+                              has_alpha ? kAlphaCodecStreams : 1));
+  }
 
   ++picture_index_;
 
@@ -192,6 +196,7 @@ int MultiplexEncoderAdapter::Release() {
   }
   encoders_.clear();
   adapter_callbacks_.clear();
+  rtc::CritScope cs(&crit_);
   for (auto& stashed_image : stashed_images_) {
     for (auto& image_component : stashed_image.second.image_components) {
       delete[] image_component.encoded_image._buffer;
@@ -214,12 +219,6 @@ EncodedImageCallback::Result MultiplexEncoderAdapter::OnEncodedImage(
     const EncodedImage& encodedImage,
     const CodecSpecificInfo* codecSpecificInfo,
     const RTPFragmentationHeader* fragmentation) {
-  const auto& stashed_image_itr = stashed_images_.find(encodedImage._timeStamp);
-  const auto& stashed_image_next_itr = std::next(stashed_image_itr, 1);
-  RTC_DCHECK(stashed_image_itr != stashed_images_.end());
-  MultiplexImage& stashed_image = stashed_image_itr->second;
-  const uint8_t frame_count = stashed_image.component_count;
-
   // Save the image
   MultiplexImageComponent image_component;
   image_component.component_index = stream_idx;
@@ -230,6 +229,13 @@ EncodedImageCallback::Result MultiplexEncoderAdapter::OnEncodedImage(
   std::memcpy(image_component.encoded_image._buffer, encodedImage._buffer,
               encodedImage._length);
 
+  rtc::CritScope cs(&crit_);
+  const auto& stashed_image_itr = stashed_images_.find(encodedImage._timeStamp);
+  const auto& stashed_image_next_itr = std::next(stashed_image_itr, 1);
+  RTC_DCHECK(stashed_image_itr != stashed_images_.end());
+  MultiplexImage& stashed_image = stashed_image_itr->second;
+  const uint8_t frame_count = stashed_image.component_count;
+
   stashed_image.image_components.push_back(image_component);
 
   if (stashed_image.image_components.size() == frame_count) {