Fix deadlock in VAAPI video decoder error cases.

content/common/gpu/media/vaapi_video_decode_accelerator.cc

 // Copyright (c) 2012 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 "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/synchronization/waitable_event.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
 #include "content/public/common/content_switches.h"
 #include "content/common/gpu/gpu_channel.h"
 #include "content/common/gpu/media/vaapi_video_decode_accelerator.h"
 #include "media/base/bind_to_loop.h"
 #include "media/video/picture.h"
 #include "third_party/libva/va/va.h"
 #include "ui/gl/gl_bindings.h"
 
 #define RETURN_AND_NOTIFY_ON_FAILURE(result, log, error_code, ret)  \
   do {                                                              \
     if (!(result)) {                                                \
       DVLOG(1) << log;                                              \
-      NotifyError(error_code);                                      \
+      message_loop_->PostTask(FROM_HERE, base::Bind(                \

[Ami GONE FROM CHROMIUM, 2012/09/18 02:21:28]

This will delay clearing client_ potentially for longer than we want.

IWBN to instead post the Cleanup() call in NotifyError, instead of posting this
here.

[sheu, 2012/09/19 08:21:35]

That would possibly be racy as well.  I imagine the client might call Destroy()
on this upon receiving the error notification, and a posted Cleanup() task
afterwards would Have A Bad Time.

In any case, this is an error case, and we might be ok with not clearing client_
for a while?

[sheu, 2012/09/19 08:58:57]

Oh!  I see what you're doing there.

Posting the cleanup with weak_this_ should do the trick.  Also, as to clearing
client_: as I understand it, the point of clearing client_ptr_factory_ is to
invalidate all the previous callbacks that haven't completed yet, since they've
all been posted with client_.

+          &VaapiVideoDecodeAccelerator::NotifyError,                \
+          weak_this_, error_code));                                 \
       return ret;                                                   \
     }                                                               \
   } while (0)
 
 using content::VaapiH264Decoder;
 
 VaapiVideoDecodeAccelerator::InputBuffer::InputBuffer() : id(0), size(0) {
 }
 
 VaapiVideoDecodeAccelerator::InputBuffer::~InputBuffer() {
 }
 
 void VaapiVideoDecodeAccelerator::NotifyError(Error error) {
-  if (message_loop_ != MessageLoop::current()) {
-    DCHECK_EQ(decoder_thread_.message_loop(), MessageLoop::current());
-    message_loop_->PostTask(FROM_HERE, base::Bind(
-        &VaapiVideoDecodeAccelerator::NotifyError, weak_this_, error));
-    return;
-  }
-
+  // Always call this from a posted task, as it calls Cleanup(), which acquires
+  // the member lock_.
   DVLOG(1) << "Notifying of error " << error;
-
   if (client_) {
     client_->NotifyError(error);
     client_ptr_factory_.InvalidateWeakPtrs();
   }
   Cleanup();
 }
 
 VaapiVideoDecodeAccelerator::VaapiVideoDecodeAccelerator(
     Display* x_display, GLXContext glx_context,
     Client* client,
@@ skipping 527 matching lines @@
 
 // static
 void VaapiVideoDecodeAccelerator::PreSandboxInitialization() {
   VaapiH264Decoder::PreSandboxInitialization();
 }
 
 // static
 bool VaapiVideoDecodeAccelerator::PostSandboxInitialization() {
   return VaapiH264Decoder::PostSandboxInitialization();
 }