VAVDA: Fix locking.

content/common/gpu/media/vaapi_video_decode_accelerator.cc

Index: content/common/gpu/media/vaapi_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/vaapi_video_decode_accelerator.cc b/content/common/gpu/media/vaapi_video_decode_accelerator.cc
index 065f8e49d9d624fc88e1c2ccd9196e81c43eeea4..d54db04d18521171113f799347a0c520417c4a57 100644
--- a/content/common/gpu/media/vaapi_video_decode_accelerator.cc
+++ b/content/common/gpu/media/vaapi_video_decode_accelerator.cc
@@ -129,13 +129,14 @@ void VaapiVideoDecodeAccelerator::MapAndQueueNewInputBuffer(
   RETURN_AND_NOTIFY_ON_FAILURE(shm->Map(bitstream_buffer.size()),
                               "Failed to map input buffer", UNREADABLE_INPUT,);
 
+  base::AutoLock auto_lock(lock_);

[Ami GONE FROM CHROMIUM, 2012/08/03 20:38:16]

This change isn't strictly necessary b/c the extra protection it affords is
around a new linked_ptr<InputBuffer> wrapping a new InputBuffer.

[piman, 2012/08/03 21:05:02]

This change is absolutely necessary (we found the issue because this wasn't
done).

If the AutoLock is below, then it will go out of scope before input_buffer. Then
when input_buffer goes out of scope, it will modify its linked list, racing with
modifications to the linked list in the other thread (e.g. GetInputBuffer, which
will assign a new linked_ptr that will also modify the linked list).

[Ami GONE FROM CHROMIUM, 2012/08/03 21:06:41]

Good point!

+
   // Set up a new input buffer and queue it for later.
   linked_ptr<InputBuffer> input_buffer(new InputBuffer());
   input_buffer->shm.reset(shm.release());
   input_buffer->id = bitstream_buffer.id();
   input_buffer->size = bitstream_buffer.size();
 
-  base::AutoLock auto_lock(lock_);
   input_buffers_.push(input_buffer);
   input_ready_.Signal();
 }
@@ -157,10 +158,15 @@ void VaapiVideoDecodeAccelerator::InitialDecodeTask() {
   for (;;) {
     if (!GetInputBuffer())
       return;
-    DCHECK(curr_input_buffer_.get());
 
-    VaapiH264Decoder::DecResult res = decoder_.DecodeInitial(
-        curr_input_buffer_->id);
+    int32 id = 0;
+    {
+      base::AutoLock auto_lock(lock_);
+      DCHECK(curr_input_buffer_.get());
+      id = curr_input_buffer_->id;
+    }
+
+    VaapiH264Decoder::DecResult res = decoder_.DecodeInitial(id);
     switch (res) {
       case VaapiH264Decoder::kReadyToDecode:
         if (state_ == kInitialized) {
@@ -247,11 +253,11 @@ bool VaapiVideoDecodeAccelerator::GetInputBuffer() {
   }
 }
 
-void VaapiVideoDecodeAccelerator::ReturnCurrInputBuffer() {
+void VaapiVideoDecodeAccelerator::ReturnCurrInputBufferLocked() {
+  lock_.AssertAcquired();
   DCHECK_EQ(decoder_thread_.message_loop(), MessageLoop::current());
-
-  base::AutoLock auto_lock(lock_);
   DCHECK(curr_input_buffer_.get());
+
   int32 id = curr_input_buffer_->id;
   curr_input_buffer_.reset();
   DVLOG(4) << "End of input buffer " << id;
@@ -259,6 +265,11 @@ void VaapiVideoDecodeAccelerator::ReturnCurrInputBuffer() {
       &Client::NotifyEndOfBitstreamBuffer, client_, id));
 }
 
+void VaapiVideoDecodeAccelerator::ReturnCurrInputBuffer() {
+  base::AutoLock auto_lock(lock_);
+  ReturnCurrInputBufferLocked();
+}
+
 bool VaapiVideoDecodeAccelerator::GetOutputBuffers() {
   DCHECK_EQ(decoder_thread_.message_loop(), MessageLoop::current());
 
@@ -292,10 +303,15 @@ void VaapiVideoDecodeAccelerator::DecodeTask() {
     if (!GetInputBuffer())
       // Early exit requested.
       return;
-    DCHECK(curr_input_buffer_.get());
 
-    VaapiH264Decoder::DecResult res =
-        decoder_.DecodeOneFrame(curr_input_buffer_->id);
+    int32 id = 0;
+    {
+      base::AutoLock auto_lock(lock_);
+      DCHECK(curr_input_buffer_.get());
+      id = curr_input_buffer_->id;
+    }
+
+    VaapiH264Decoder::DecResult res = decoder_.DecodeOneFrame(id);
     switch (res) {
       case VaapiH264Decoder::kNeedMoreStreamData:
         ReturnCurrInputBuffer();
@@ -454,9 +470,11 @@ void VaapiVideoDecodeAccelerator::ResetTask() {
   // to call Decode() after Reset() and before NotifyResetDone.
   decoder_.Reset();
 
+  base::AutoLock auto_lock(lock_);
+
   // Return current input buffer, if present.
   if (curr_input_buffer_.get())
-    ReturnCurrInputBuffer();
+    ReturnCurrInputBufferLocked();
 
   // And let client know that we are done with reset.
   message_loop_->PostTask(FROM_HERE, base::Bind(