Convert AVDAs thread hang detection to be timer based

media/gpu/android_video_decode_accelerator.cc

Index: media/gpu/android_video_decode_accelerator.cc
diff --git a/media/gpu/android_video_decode_accelerator.cc b/media/gpu/android_video_decode_accelerator.cc
index 41d3f550768b03219f98c57608a678c0e74ca8ad..dd059a647da062924f311f3da25d16200a56ae9f 100644
--- a/media/gpu/android_video_decode_accelerator.cc
+++ b/media/gpu/android_video_decode_accelerator.cc
@@ -16,6 +16,7 @@
 #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"
@@ -105,6 +106,9 @@ constexpr base::TimeDelta IdleTimerTimeOut = base::TimeDelta::FromSeconds(1);
 // from breaking the pipeline, if we're about to be reset anyway.
 constexpr base::TimeDelta ErrorPostingDelay = base::TimeDelta::FromSeconds(2);
 
+constexpr base::TimeDelta kHungTaskDetectionTimeout =
+    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 };
@@ -184,6 +188,41 @@ class AndroidVideoDecodeAccelerator::OnFrameAvailableHandler
 //    surfaces are released.
 class AVDAManager {
  public:
+  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_.reset();
+    }
+
+    void Reset() {
+      base::AutoLock l(lock_);
+      task_start_time_.reset();
+    }
+
+    bool ThreadLikelyHung() {
+      base::AutoLock l(lock_);
+      if (task_start_time_.has_value()) {

[DaleCurtis, 2016/08/17 04:58:45]

is_null() is sufficient, ticks never starts at zero in practice.

+        return (base::TimeTicks::Now() - task_start_time_.value()) >
+               kHungTaskDetectionTimeout;
+      }
+      return false;
+    }
+
+   private:
+    base::Lock lock_;
+    base::Optional<base::TimeTicks> task_start_time_;
+
+    DISALLOW_COPY_AND_ASSIGN(HangDetector);
+  };
+
   // Make sure that the construction thread is started for |avda|.
   bool StartThread(AndroidVideoDecodeAccelerator* avda) {
     DCHECK(thread_checker_.CalledOnValidThread());
@@ -196,6 +235,11 @@ class AVDAManager {
         LOG(ERROR) << "Failed to start construction thread.";
         return false;
       }
+      construction_thread_.task_runner()->PostTask(
+          FROM_HERE,
+          base::Bind(&base::MessageLoop::AddTaskObserver,
+                     base::Unretained(construction_thread_.message_loop()),
+                     &hang_detector_));
     }
 
     thread_avda_instances_.insert(avda);
@@ -214,14 +258,8 @@ class AVDAManager {
     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();
+    // TODO: post a task to stop the thread.

[liberato (no reviews please), 2016/08/17 15:15:42]

not sure that posting a task to stop the thread is all that simple.  seems like
it'd be hard to guarantee that the thread is re-started properly if the shutdown
is racing with a future post that believes that it's already started.

+    hang_detector_.Reset();

[liberato (no reviews please), 2016/08/17 15:15:42]

i don't think that it should reset here.  for example, if we destroy the only
avda instance while an async codec config is hung, then we don't want to start
posting tasks again.  if it does eventually complete, it'll reset itself
normally even if the "AndReply" part cancels due to the invalidated weak ptr.

   }
 
   // Request periodic callback of |avda|->DoIOTask(). Does nothing if the
@@ -264,33 +302,9 @@ class AVDAManager {
     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;
+    return !hang_detector_.ThreadLikelyHung();
   }
 
   // |avda| would like to use |surface_id|.  If it is not busy, then mark it
@@ -398,16 +412,8 @@ class AVDAManager {
   // 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_;
-
   base::Thread construction_thread_;
+  HangDetector hang_detector_;
 
   base::ThreadChecker thread_checker_;
 
@@ -1110,10 +1116,6 @@ void AndroidVideoDecodeAccelerator::ConfigureMediaCodecAsynchronously() {
     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 =
@@ -1139,7 +1141,6 @@ bool AndroidVideoDecodeAccelerator::ConfigureMediaCodecSynchronously() {
   // 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 =
@@ -1168,11 +1169,6 @@ AndroidVideoDecodeAccelerator::ConfigureMediaCodecOnAnyThread(
       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;
 }
 
@@ -1683,13 +1679,8 @@ void AndroidVideoDecodeAccelerator::ReleaseMediaCodec() {
   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());
   }
 }