Convert AVDAs thread hang detection to be timer based

media/gpu/android_video_decode_accelerator.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/gpu/android_video_decode_accelerator.h"
 
 #include <stddef.h>
 
 #include <memory>
 
 #include "base/android/build_info.h"
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
+#include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/task_runner_util.h"
 #include "base/threading/thread_checker.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 #include "gpu/command_buffer/service/mailbox_manager.h"
 #include "gpu/ipc/service/gpu_channel.h"
 #include "media/base/android/media_codec_bridge.h"
 #include "media/base/android/media_codec_util.h"
@@ skipping 69 matching lines @@
 
 constexpr base::TimeDelta NoWaitTimeOut = base::TimeDelta::FromMicroseconds(0);
 
 constexpr base::TimeDelta IdleTimerTimeOut = base::TimeDelta::FromSeconds(1);
 
 // Time between when we notice an error, and when we actually notify somebody.
 // This is to prevent codec errors caused by SurfaceView fullscreen transitions
 // from breaking the pipeline, if we're about to be reset anyway.
 constexpr base::TimeDelta ErrorPostingDelay = base::TimeDelta::FromSeconds(2);
 
-// Maximum number of concurrent, incomplete codec creations that we'll allow
-// before turning off autodection of codec type.
-enum { kMaxConcurrentCodecAutodetections = 4 };
+// Give tasks on the construction thread 800ms before considering them hung.
+// MediaCodec.configure() calls typically take 100-200ms on a N5, so 800ms is
+// expected to very rarely result in false positives. Also, false positives have
+// low impact because we resume using the thread if its apparently hung task
+// completes.
+constexpr base::TimeDelta kHungTaskDetectionTimeout =
+    base::TimeDelta::FromMilliseconds(800);
 
 // For RecordFormatChangedMetric.
 enum FormatChangedValue {
   CodecInitialized = false,
   MissingFormatChanged = true
 };
 
 inline void RecordFormatChangedMetric(FormatChangedValue value) {
   UMA_HISTOGRAM_BOOLEAN("Media.AVDA.MissingFormatChanged", !!value);
 }
@@ skipping 48 matching lines @@
   // AVDA that wants the OnFrameAvailable callback.
   AndroidVideoDecodeAccelerator* owner_;
 
   DISALLOW_COPY_AND_ASSIGN(OnFrameAvailableHandler);
 };
 
 // AVDAManager manages shared resources for a number of AVDA instances.
 // Its responsibilities include:
 //  - Starting and stopping a shared "construction" thread for instantiating and
 //    releasing MediaCodecs.
-//  - Tracking the number of outstanding tasks running on the construction
-//    thread. (For detecting when one of those tasks has hung indefinitely.)
+//  - Detecting when a task has hung on the construction thread so AVDAs can
+//    stop using it.
 //  - Running a RepeatingTimer so that AVDAs can get a regular callback to
 //    DoIOTask().
 //  - Tracking the allocation of surfaces to AVDAs and delivering callbacks when
 //    surfaces are released.
 class AVDAManager {
  public:
-  // Make sure that the construction thread is started for |avda|.
+  class HangDetector : public base::MessageLoop::TaskObserver {
+   public:
+    HangDetector() {}
+
+    void WillProcessTask(const base::PendingTask& pending_task) override {
+      base::AutoLock l(lock_);
+      task_start_time_ = base::TimeTicks::Now();
+    }
+
+    void DidProcessTask(const base::PendingTask& pending_task) override {
+      base::AutoLock l(lock_);
+      task_start_time_ = base::TimeTicks();
+    }
+
+    bool ThreadLikelyHung() {

[DaleCurtis, 2016/08/19 22:56:03]

IsThreadLikelyHung?

[watk, 2016/08/19 23:22:28]

Done.

+      base::AutoLock l(lock_);
+      if (task_start_time_.is_null())
+        return false;
+
+      return (base::TimeTicks::Now() - task_start_time_) >
+             kHungTaskDetectionTimeout;
+    }
+
+   private:
+    base::Lock lock_;
+    // Non-null when a task is currently running.
+    base::TimeTicks task_start_time_;
+
+    DISALLOW_COPY_AND_ASSIGN(HangDetector);
+  };
+
+  // Make sure the construction thread is started for |avda|.
   bool StartThread(AndroidVideoDecodeAccelerator* avda) {
     DCHECK(thread_checker_.CalledOnValidThread());
 
-    // If we chose not to shut it down due to pending codec constructions, then
-    // the thread might already be started even if there are no avda instances.
-    // Plus, sometimes we just fail to start the thread.
     if (!construction_thread_.IsRunning()) {
       if (!construction_thread_.Start()) {
         LOG(ERROR) << "Failed to start construction thread.";
         return false;
       }
+      // Register |hang_detector_| to observe the thread's MessageLoop.
+      construction_thread_.task_runner()->PostTask(
+          FROM_HERE,
+          base::Bind(&base::MessageLoop::AddTaskObserver,
+                     base::Unretained(construction_thread_.message_loop()),
+                     &hang_detector_));
     }
 
+    // Cancel any pending StopThreadTask()s because we need the thread now.
+    weak_this_factory_.InvalidateWeakPtrs();
     thread_avda_instances_.insert(avda);
     UMA_HISTOGRAM_ENUMERATION("Media.AVDA.NumAVDAInstances",
                               thread_avda_instances_.size(),
                               31);  // PRESUBMIT_IGNORE_UMA_MAX
     return true;
   }
 
-  // |avda| will no longer need the construction thread.  Stop the thread if
-  // this is the last instance.
   void StopThread(AndroidVideoDecodeAccelerator* avda) {
     DCHECK(thread_checker_.CalledOnValidThread());
 
     thread_avda_instances_.erase(avda);
-    if (!thread_avda_instances_.empty())
-      return;
-
-    // Don't stop the thread if there are outstanding requests, since they
-    // might be hung.  They also might simply be incomplete, and the thread
-    // will stay running until we try to shut it down again.
-    base::AutoLock auto_lock(autodetection_info_.lock_);
-    if (autodetection_info_.outstanding_)
-      return;
-
-    construction_thread_.Stop();
+    // Post a task to stop the thread through the thread's task runner and back
+    // to this thread. This ensures that all pending tasks are run first. If the
+    // thread is hung we don't post a task to avoid leaking an unbounded number
+    // of tasks on its queue. If the thread is not hung, but appears to be, it
+    // will stay alive until next time an AVDA tries to stop it.
+    if (thread_avda_instances_.empty() && !hang_detector_.ThreadLikelyHung()) {
+      construction_thread_.task_runner()->PostTaskAndReply(

[DaleCurtis, 2016/08/19 22:56:03]

Clever!

+          FROM_HERE, base::Bind(&base::DoNothing),
+          base::Bind(&AVDAManager::StopThreadTask,
+                     weak_this_factory_.GetWeakPtr()));
+    }
   }
 
   // Request periodic callback of |avda|->DoIOTask(). Does nothing if the
   // instance is already registered and the timer started. The first request
   // will start the repeating timer on an interval of DecodePollDelay.
   void StartTimer(AndroidVideoDecodeAccelerator* avda) {
     DCHECK(thread_checker_.CalledOnValidThread());
 
     timer_avda_instances_.insert(avda);
 
@@ skipping 22 matching lines @@
     timer_avda_instances_.erase(avda);
     if (timer_avda_instances_.empty())
       io_timer_.Stop();
   }
 
   scoped_refptr<base::SingleThreadTaskRunner> ConstructionTaskRunner() {
     DCHECK(thread_checker_.CalledOnValidThread());
     return construction_thread_.task_runner();
   }
 
-  // Called on the main thread when the construction thread will be doing work
-  // that can potentially hang (e.g., autodetection).  There may be several
-  // calls to this before any call to DoneUsingConstructionThread.
-  // Note that this should only be called from the main thread, else it's a race
-  // with IsCodecAutodetectionProbablySafe.
-  void StartUsingConstructionThread() {
-    DCHECK(thread_checker_.CalledOnValidThread());
-    base::AutoLock auto_lock(autodetection_info_.lock_);
-    ++autodetection_info_.outstanding_;
-  }
-
-  // Called on any thread after the potentially dangerous construction thread
-  // work completes safely.  May be called on any thread, including the
-  // construction thread.
-  // This assumes that requests are ordered, so please don't mix sync and async
-  // codec construction here.
-  void DoneUsingConstructionThread() {
-    base::AutoLock auto_lock_l(autodetection_info_.lock_);
-    DCHECK_GT(autodetection_info_.outstanding_, 0);
-    --autodetection_info_.outstanding_;
-  }
-
   // Return a hint about whether autodetecting the codec type is safe or not.
   bool IsCodecAutodetectionProbablySafe() {
-    base::AutoLock auto_lock_l(autodetection_info_.lock_);
-
-    return autodetection_info_.outstanding_ < kMaxConcurrentCodecAutodetections;
+    DCHECK(thread_checker_.CalledOnValidThread());
+    return !hang_detector_.ThreadLikelyHung();
   }
 
   // |avda| would like to use |surface_id|.  If it is not busy, then mark it
   // as busy and return true.  If it is busy, then replace any existing waiter,
   // make |avda| the current waiter, and return false.  Any existing waiter
   // is assumed to be on the way out, so we fail its allocation request.
   bool AllocateSurface(int surface_id, AndroidVideoDecodeAccelerator* avda) {
     // Nobody has to wait for no surface.
     if (surface_id == AndroidVideoDecodeAccelerator::Config::kNoSurfaceID)
       return true;
@@ skipping 43 matching lines @@
 
     // Promote |waiter| to be the owner.
     iter->second.owner = waiter;
     iter->second.waiter = nullptr;
     waiter->OnSurfaceAvailable(true);
   }
 
  private:
   friend struct base::DefaultLazyInstanceTraits<AVDAManager>;
 
-  AVDAManager() : construction_thread_("AVDAThread") {}
+  AVDAManager()
+      : construction_thread_("AVDAThread"), weak_this_factory_(this) {}
   ~AVDAManager() { NOTREACHED(); }
 
   void RunTimer() {
     {
       // Call out to all AVDA instances, some of which may attempt to remove
       // themselves from the list during this operation; those removals will be
       // deferred until after all iterations are complete.
       base::AutoReset<bool> scoper(&timer_running_, true);
       for (auto* avda : timer_avda_instances_)
         avda->DoIOTask(false);
     }
 
     // Take care of any deferred erasures.
     for (auto* avda : pending_erase_)
       StopTimer(avda);
     pending_erase_.clear();
 
     // TODO(dalecurtis): We may want to consider chunking this if task execution
     // takes too long for the combined timer.
   }
 
+  void StopThreadTask() { construction_thread_.Stop(); }
+
+  // All registered AVDA instances.
+  std::set<AndroidVideoDecodeAccelerator*> thread_avda_instances_;
+
   // All AVDA instances that would like us to poll DoIOTask.
   std::set<AndroidVideoDecodeAccelerator*> timer_avda_instances_;
 
-  // All AVDA instances that might like to use the construction thread.
-  std::set<AndroidVideoDecodeAccelerator*> thread_avda_instances_;
-
   struct OwnerRecord {
     AndroidVideoDecodeAccelerator* owner = nullptr;
     AndroidVideoDecodeAccelerator* waiter = nullptr;
   };
   // [surface id] = OwnerRecord for that surface.
   using SurfaceWaiterMap = std::map<int, OwnerRecord>;
   SurfaceWaiterMap surface_waiter_map_;
 
   // Since we can't delete while iterating when using a set, defer erasure until
   // after iteration complete.
   bool timer_running_ = false;
   std::set<AndroidVideoDecodeAccelerator*> pending_erase_;
 
   // Repeating timer responsible for draining pending IO to the codecs.
   base::RepeatingTimer io_timer_;
 
-  // Data for determining if codec creation is hanging.
-  struct {
-    // Lock that protects other members of this struct.
-    base::Lock lock_;
-
-    // Number of currently pending work items of the construction thread.
-    int outstanding_ = 0;
-  } autodetection_info_;
-
+  // A thread for posting MediaCodec construction and releases to. It's created
+  // lazily when requested.
   base::Thread construction_thread_;
 
+  // For detecting when MediaCodec a task has hung on |construction_thread_|.
+  HangDetector hang_detector_;
+
   base::ThreadChecker thread_checker_;
 
+  // For canceling pending StopThreadTask()s.
+  base::WeakPtrFactory<AVDAManager> weak_this_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(AVDAManager);
 };
 
 static base::LazyInstance<AVDAManager>::Leaky g_avda_manager =
     LAZY_INSTANCE_INITIALIZER;
 
 AndroidVideoDecodeAccelerator::CodecConfig::CodecConfig() {}
 
 AndroidVideoDecodeAccelerator::CodecConfig::~CodecConfig() {}
 
@@ skipping 155 matching lines @@
       on_destroying_surface_cb_);
 
   // TODO(watk,liberato): move this into the strategy.
   scoped_refptr<gl::SurfaceTexture> surface_texture =
       strategy_->GetSurfaceTexture();
   if (surface_texture) {
     on_frame_available_handler_ =
         new OnFrameAvailableHandler(this, surface_texture);
   }
 
-  // Start the thread for async configuration, even if we don't need it now.
-  // ResetCodecState might rebuild the codec later, for example.
-  if (!g_avda_manager.Get().StartThread(this)) {
-    LOG(ERROR) << "Failed to start AVDA thread";
+  if (!g_avda_manager.Get().StartThread(this))
     return false;
-  }
 
   // If we are encrypted, then we aren't able to create the codec yet.
   if (config_.is_encrypted) {
     InitializeCdm();
     return true;
   }
 
   if (deferred_initialization_pending_) {
     ConfigureMediaCodecAsynchronously();
     return true;
@@ skipping 498 matching lines @@
       g_avda_manager.Get().IsCodecAutodetectionProbablySafe();
 
   // If autodetection is disallowed, fall back to Chrome's software decoders
   // instead of using the software decoders provided by MediaCodec.
   if (!codec_config_->allow_autodetection_ &&
       IsMediaCodecSoftwareDecodingForbidden()) {
     OnCodecConfigured(nullptr);
     return;
   }
 
-  codec_config_->notify_completion_ = codec_config_->allow_autodetection_;
-  if (codec_config_->allow_autodetection_)
-    g_avda_manager.Get().StartUsingConstructionThread();
-
   // If we're not trying autodetection, then use the main thread.  The original
   // might be blocked.
   scoped_refptr<base::SingleThreadTaskRunner> task_runner =
       codec_config_->allow_autodetection_
           ? g_avda_manager.Get().ConstructionTaskRunner()
           : base::ThreadTaskRunnerHandle::Get();
   CHECK(task_runner);
 
   base::PostTaskAndReplyWithResult(
       task_runner.get(), FROM_HERE,
       base::Bind(&AndroidVideoDecodeAccelerator::ConfigureMediaCodecOnAnyThread,
                  codec_config_),
       base::Bind(&AndroidVideoDecodeAccelerator::OnCodecConfigured,
                  weak_this_factory_.GetWeakPtr()));
 }

[DaleCurtis, 2016/08/19 22:56:03]

Shouldn't this be based on the IsLikelyHung method now?

[watk, 2016/08/19 23:22:28]

Yep, the IsCodecAutodetectionProbablySafe() on l.1117 above calls it.

 
 bool AndroidVideoDecodeAccelerator::ConfigureMediaCodecSynchronously() {
   state_ = WAITING_FOR_CODEC;
 
-  // Decide whether to allow autodetection or not.  Since we're on the main
-  // thread, and this request is unordered with respect to pending async config
-  // attempts, don't record it.  It may break book-keeping, and there's not
-  // much we can do anyway.
   codec_config_->allow_autodetection_ =
       g_avda_manager.Get().IsCodecAutodetectionProbablySafe();
-  codec_config_->notify_completion_ = false;
 
   ReleaseMediaCodec();
   std::unique_ptr<VideoCodecBridge> media_codec =
       ConfigureMediaCodecOnAnyThread(codec_config_);
   OnCodecConfigured(std::move(media_codec));
   return !!media_codec_;
 }
 
 std::unique_ptr<VideoCodecBridge>
 AndroidVideoDecodeAccelerator::ConfigureMediaCodecOnAnyThread(
     scoped_refptr<CodecConfig> codec_config) {
   TRACE_EVENT0("media", "AVDA::ConfigureMediaCodec");
 
   jobject media_crypto = codec_config->media_crypto_
                              ? codec_config->media_crypto_->obj()
                              : nullptr;
 
   // |needs_protected_surface_| implies encrypted stream.
   DCHECK(!codec_config->needs_protected_surface_ || media_crypto);
 
   const bool require_software_codec = !codec_config->allow_autodetection_;
 
   std::unique_ptr<VideoCodecBridge> codec(VideoCodecBridge::CreateDecoder(
       codec_config->codec_, codec_config->needs_protected_surface_,
       codec_config->initial_expected_coded_size_,
       codec_config->surface_.j_surface().obj(), media_crypto, true,
       require_software_codec));
 
-  // If we successfully completed after an autodetect, then let the other
-  // instances know that we didn't get stuck.
-  if (codec_config->notify_completion_)
-    g_avda_manager.Get().DoneUsingConstructionThread();
-
   return codec;
 }
 
 void AndroidVideoDecodeAccelerator::OnCodecConfigured(
     std::unique_ptr<VideoCodecBridge> media_codec) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(state_ == WAITING_FOR_CODEC || state_ == SURFACE_DESTROYED);
 
   // Record one instance of the codec being initialized.
   RecordFormatChangedMetric(FormatChangedValue::CodecInitialized);
@@ skipping 490 matching lines @@
   // backed by hardware, else it may hang too.  Post it to the construction
   // thread, and it'll get freed if things start working.  If things are
   // already working, then it'll be freed soon.
   //
   // We require software codecs when |allow_autodetection_| is false, so use
   // the stored value as a proxy for whether the MediaCodec is software backed
   // or not.
   if (!codec_config_->allow_autodetection_) {
     media_codec_.reset();
   } else {
-    g_avda_manager.Get().StartUsingConstructionThread();
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner =
-        g_avda_manager.Get().ConstructionTaskRunner();
-    task_runner->DeleteSoon(FROM_HERE, media_codec_.release());
-    task_runner->PostTask(
-        FROM_HERE, base::Bind(&AVDAManager::DoneUsingConstructionThread,
-                              base::Unretained(g_avda_manager.Pointer())));
+    g_avda_manager.Get().ConstructionTaskRunner()->DeleteSoon(
+        FROM_HERE, media_codec_.release());
   }
 }
 
 // static
 bool AndroidVideoDecodeAccelerator::UseDeferredRenderingStrategy(
     const gpu::GpuPreferences& gpu_preferences) {
   return true;
 }
 
 // static
@@ skipping 78 matching lines @@
 
 bool AndroidVideoDecodeAccelerator::IsMediaCodecSoftwareDecodingForbidden()
     const {
   // Prevent MediaCodec from using its internal software decoders when we have
   // more secure and up to date versions in the renderer process.
   return !config_.is_encrypted && (codec_config_->codec_ == media::kCodecVP8 ||
                                    codec_config_->codec_ == media::kCodecVP9);
 }
 
 }  // namespace media