chromeos: Retry ALSA mixer initialization on failure.

chrome/browser/chromeos/audio_mixer_alsa.cc

Index: chrome/browser/chromeos/audio_mixer_alsa.cc
diff --git a/chrome/browser/chromeos/audio_mixer_alsa.cc b/chrome/browser/chromeos/audio_mixer_alsa.cc
index a84ffeb519be7b51dc30805aebca216a9f8675a6..4b324de4048d5cd93e09ac0ad35eeb4d9c679e9a 100644
--- a/chrome/browser/chromeos/audio_mixer_alsa.cc
+++ b/chrome/browser/chromeos/audio_mixer_alsa.cc
@@ -1,10 +1,11 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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 "chrome/browser/chromeos/audio_mixer_alsa.h"
 
 #include <cmath>
+#include <unistd.h>
 
 #include <alsa/asoundlib.h>
 
@@ -40,6 +41,14 @@ const int kPrefMuteOff = 0;
 const int kPrefMuteOn = 1;
 const int kPrefMuteInvalid = 2;
 
+// Maximum number of times that we'll attempt to initialize the mixer.
+// We'll fail until the ALSA modules have been loaded; see
+// http://crosbug.com/13162.
+const int kMaxInitAttempts = 20;
+
+// Number of seconds that we'll sleep between each initialization attempt.
+const int kInitRetrySleepSec = 1;
+
 }  // namespace
 
 AudioMixerAlsa::AudioMixerAlsa()
@@ -198,7 +207,14 @@ void AudioMixerAlsa::RegisterPrefs(PrefService* local_state) {
 // Private functions follow
 
 void AudioMixerAlsa::DoInit(InitDoneCallback* callback) {
-  bool success = InitializeAlsaMixer();
+  bool success = false;
+  for (int num_attempts = 0; num_attempts < kMaxInitAttempts; ++num_attempts) {
+    success = InitializeAlsaMixer();
+    if (success)
+      break;
+    else
+      sleep(kInitRetrySleepSec);
+  }

[davejcool, 2011/03/29 17:50:55]

There's a case where InitializeAlsaMixer() will return false if mixer_state_ !=
INITIALIZING.  Perhaps we should check that here with something like:

    } else {
      base::AutoLock lock(mixer_state_lock_);
      if (mixer_state_ != INITIALIZING)
        break;
      sleep(kInitRetrySleepSec);
    }

This is because the other thread could tell us to shut down, and in that case we
can stop trying to initialize.

[Daniel Erat, 2011/03/29 19:45:55]

Thanks, that's a good point.  But holding the lock while we sleep is going to
block the UI thread, right?

I have a related question: what's the purpose of the IN_ERROR state?  It only
gets set in GetState(), and only if mixer_state_ is READY but alsa_mixer_ is
NULL, but I don't see how we can arrive at that condition:

- mixer_state_ is only set to READY in InitializeAlsaMixer(), at the same time
as when we set alsa_mixer_ (presumably to a non-NULL value)
- alsa_mixer_ only gets reset to NULL in FreeAlsaMixer(), which is only called
by the d'tor after we've set mixer_state_ to SHUTTING_DOWN
- if the code run in the order (d'tor sets SHUTTING_DOWN, InitializeAlsaMixer()
runs, FreeAlsaMixer() runs), InitializeAlsaMixer() will bail out when it sees
the state without modifying either mixer_state_ or alsa_mixer_

So I don't see how we can end up with both READY and NULL.  Am I missing
something?

[davejcool, 2011/03/30 17:42:15]

Ah, right - for this to work sleep() should be done outside the lock.  With
that, I think it should stop trying to initialize properly if the UI thread
started a shutdown.

And I think you're right about the IN_ERROR state, it's not used anymore.  With
PulseAudio, we could lose connection with the mixer and the handle could be set
to NULL.  Now there's no way for that to happen.  So IN_ERROR can be pulled out
altogether.

 
   if (success) {
     BrowserThread::PostTask(