VAVDA: Fix locking.

VAVDA: Fix locking.

Ensure proper locking around all uses of curr_input_buffer_ and input_buffers_.

TEST=by hand
BUG=none

Change-Id: I4cc92a1b0be47ce95bc0def580a88ac3a9f7deec


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149973

[piman, 2012-08-03 20:32:28]

Some background because it may not be obvious: linked_ptr isn't thread-safe
(since it's basically a linked list) so every time any linked_ptr pointing to
the same instance is operated on (including if that particular linked_ptr
instance is only accessed from a single thread), the lock must be held.

[piman, 2012-08-03 20:34:00]

lgtm

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

LGTM 
(comment is only informative)

https://chromiumcodereview.appspot.com/10853005/diff/1/content/common/gpu/med...
File content/common/gpu/media/vaapi_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/10853005/diff/1/content/common/gpu/med...
content/common/gpu/media/vaapi_video_decode_accelerator.cc:132: base::AutoLock
auto_lock(lock_);
This change isn't strictly necessary b/c the extra protection it affords is
around a new linked_ptr<InputBuffer> wrapping a new InputBuffer.

[Pawel Osciak, 2012-08-03 20:47:10]

lgtm

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

https://chromiumcodereview.appspot.com/10853005/diff/1/content/common/gpu/med...
File content/common/gpu/media/vaapi_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/10853005/diff/1/content/common/gpu/med...
content/common/gpu/media/vaapi_video_decode_accelerator.cc:132: base::AutoLock
auto_lock(lock_);
On 2012/08/03 20:38:16, Ami Fischman wrote:
> This change isn't strictly necessary b/c the extra protection it affords is
> around a new linked_ptr<InputBuffer> wrapping a new InputBuffer.

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).

[commit-bot: I haz the power, 2012-08-03 21:05:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marcheu@chromium.org/10853005/1

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

https://chromiumcodereview.appspot.com/10853005/diff/1/content/common/gpu/med...
File content/common/gpu/media/vaapi_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/10853005/diff/1/content/common/gpu/med...
content/common/gpu/media/vaapi_video_decode_accelerator.cc:132: base::AutoLock
auto_lock(lock_);
On 2012/08/03 21:05:02, piman wrote:
> On 2012/08/03 20:38:16, Ami Fischman wrote:
> > This change isn't strictly necessary b/c the extra protection it affords is
> > around a new linked_ptr<InputBuffer> wrapping a new InputBuffer.
> 
> 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).

Good point!

[commit-bot: I haz the power, 2012-08-03 23:43:53]

Change committed as 149973