Fix race condition caused by r114084.

media/audio/audio_output_controller.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 "media/audio/audio_output_controller.h"
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/message_loop.h"
 #include "base/synchronization/waitable_event.h"
@@ skipping 31 matching lines @@
 
   if (!message_loop_.get() || message_loop_->BelongsToCurrentThread()) {
     DoStopCloseAndClearStream(NULL);
   } else {
     // http://crbug.com/120973
     base::ThreadRestrictions::ScopedAllowWait allow_wait;
     WaitableEvent completion(true /* manual reset */,
                              false /* initial state */);
     message_loop_->PostTask(FROM_HERE,
         base::Bind(&AudioOutputController::DoStopCloseAndClearStream,
-                   base::Unretained(this),
+                   this,

[tommi (sloooow) - chröme, 2012/05/02 07:56:18]

I don't think it's a good idea to do this in this particular case.
You're running in the destructor, the reference count is 0.  If you addref and
release again, you will attempt double delete.

                    &completion));
     completion.Wait();
   }
 }
 
 // static
 scoped_refptr<AudioOutputController> AudioOutputController::Create(
     AudioManager* audio_manager,
     EventHandler* event_handler,
     const AudioParameters& params,
@@ skipping 11 matching lines @@
   controller->message_loop_ = audio_manager->GetMessageLoop();
   controller->message_loop_->PostTask(FROM_HERE, base::Bind(
       &AudioOutputController::DoCreate, controller,
       base::Unretained(audio_manager), params));
   return controller;
 }
 
 void AudioOutputController::Play() {
   DCHECK(message_loop_);
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &AudioOutputController::DoPlay, base::Unretained(this)));
+      &AudioOutputController::DoPlay, this));

[tommi (sloooow) - chröme, 2012/05/02 07:56:18]

The original reason for using Unretained() here was to avoid shifting references
to the audiomanager's thread.  If the final reference is released on that
thread, then there were problems.

If memory serves, the problem was that the AudioManager itself could
subsequently have its last reference released, and hang since it would try to
shut down the audio thread from the audio thread itself.

However, I did make a change a while ago to make the manager non-ref counted, so
I wonder if that problem has gone away?  I'm not sure.

Alternatively, could you use weak_this_.GetWeakPtr() here instead?

 }
 
 void AudioOutputController::Pause() {
   DCHECK(message_loop_);
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &AudioOutputController::DoPause, base::Unretained(this)));
+      &AudioOutputController::DoPause, this));
 }
 
 void AudioOutputController::Flush() {
   DCHECK(message_loop_);
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &AudioOutputController::DoFlush, base::Unretained(this)));
+      &AudioOutputController::DoFlush, this));
 }
 
 void AudioOutputController::Close(const base::Closure& closed_task) {
   DCHECK(!closed_task.is_null());
   DCHECK(message_loop_);
   message_loop_->PostTaskAndReply(FROM_HERE, base::Bind(
-      &AudioOutputController::DoClose, base::Unretained(this)), closed_task);
+      &AudioOutputController::DoClose, this), closed_task);
 }
 
 void AudioOutputController::SetVolume(double volume) {
   DCHECK(message_loop_);
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &AudioOutputController::DoSetVolume, base::Unretained(this), volume));
+      &AudioOutputController::DoSetVolume, this, volume));
 }
 
 void AudioOutputController::DoCreate(AudioManager* audio_manager,
                                      const AudioParameters& params) {
   DCHECK(message_loop_->BelongsToCurrentThread());
 
   // Close() can be called before DoCreate() is executed.
   if (state_ == kClosed)
     return;
   DCHECK_EQ(kEmpty, state_);
@@ skipping 187 matching lines @@
     do {
       base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(1));
     } while (!sync_reader_->DataReady() &&
              Time::Now() - start_time < kMaxPollingDelay);
   }
 }
 
 void AudioOutputController::OnError(AudioOutputStream* stream, int code) {
   // Handle error on the audio controller thread.
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &AudioOutputController::DoReportError, base::Unretained(this), code));
+      &AudioOutputController::DoReportError, this, code));
 }
 
 void AudioOutputController::DoStopCloseAndClearStream(WaitableEvent *done) {
   DCHECK(message_loop_->BelongsToCurrentThread());
 
   // Allow calling unconditionally and bail if we don't have a stream_ to close.
   if (stream_ != NULL) {
     stream_->Stop();
     stream_->Close();
     stream_ = NULL;
     weak_this_.InvalidateWeakPtrs();
   }
 
   // Should be last in the method, do not touch "this" from here on.
   if (done != NULL)
     done->Signal();
 }
 
 }  // namespace media