Commit 9e60666b authored by Dale Curtis's avatar Dale Curtis Committed by Commit Bot

Revert "Add debug logging to PulseAudio startup when using ASAN/TSAN."

This reverts commit aedef3b1.

Reason for revert: Logs found the problem. pa_context_connect() is hanging.

Original change's description:
> Add debug logging to PulseAudio startup when using ASAN/TSAN.
> 
> I've been unable to trigger this issue with multiple rounds of CQ
> attempts, so I need some logs checked in to progress further.
> 
> Bug: 1047655
> Change-Id: I4329f76044687ab8dbb521491ead430b15de581d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2048039
> Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
> Commit-Queue: John Rummell <jrummell@chromium.org>
> Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: John Rummell <jrummell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#740124}

TBR=dalecurtis@chromium.org,jrummell@chromium.org

Change-Id: Ieda3b9b8c8e6ee06beb2eb9ceecb1cce2de915bb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1047655
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2050871Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740353}
parent 647cc175
......@@ -15,11 +15,6 @@ if (use_pulseaudio && !link_pulseaudio) {
sigs = [ "pulse/pulse.sigs" ]
output_name = "pulse/pulse_stubs"
deps = [ "//base" ]
# TODO(dalecurtis): Remove once https://crbug.com/1047655 is solved.
if (is_asan || is_tsan) {
logging_function = "VLOG(0)"
}
}
}
......
......@@ -21,13 +21,6 @@
#include "media/audio/pulse/pulse_util.h"
#endif
// TODO(dalecurtis): Remove once https://crbug.com/1047655 is solved.
#if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER)
#define DVLOG_LEVEL 0
#else
#define DVLOG_LEVEL 1
#endif
namespace media {
enum LinuxAudioIO {
......@@ -40,7 +33,6 @@ enum LinuxAudioIO {
std::unique_ptr<media::AudioManager> CreateAudioManager(
std::unique_ptr<AudioThread> audio_thread,
AudioLogFactory* audio_log_factory) {
DVLOG(DVLOG_LEVEL) << __func__;
#if defined(USE_CRAS)
if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kUseCras)) {
UMA_HISTOGRAM_ENUMERATION("Media.LinuxAudioIO", kCras, kAudioIOMax + 1);
......
......@@ -26,13 +26,6 @@ using media_audio_pulse::InitializeStubs;
using media_audio_pulse::StubPathMap;
#endif // defined(DLOPEN_PULSEAUDIO)
// TODO(dalecurtis): Remove once https://crbug.com/1047655 is solved.
#if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER)
#define DVLOG_LEVEL 0
#else
#define DVLOG_LEVEL 1
#endif
namespace media {
namespace pulse {
......@@ -194,14 +187,13 @@ void SignalReadyOrErrorStateCallback(pa_context* context, void* context_data) {
} // namespace
bool InitPulse(pa_threaded_mainloop** mainloop, pa_context** context) {
DVLOG(DVLOG_LEVEL) << __func__;
#if defined(DLOPEN_PULSEAUDIO)
StubPathMap paths;
// Check if the pulse library is available.
paths[kModulePulse].push_back(kPulseLib);
if (!InitializeStubs(paths)) {
DVLOG(0) << "Failed on loading the Pulse library and symbols";
VLOG(1) << "Failed on loading the Pulse library and symbols";
return false;
}
#endif // defined(DLOPEN_PULSEAUDIO)
......@@ -211,12 +203,10 @@ bool InitPulse(pa_threaded_mainloop** mainloop, pa_context** context) {
// Create a mainloop API and connect to the default server.
// The mainloop is the internal asynchronous API event loop.
DVLOG(DVLOG_LEVEL) << __func__ << ": pa_threaded_mainloop_new";
pa_threaded_mainloop* pa_mainloop = pa_threaded_mainloop_new();
if (!pa_mainloop)
return false;
DVLOG(DVLOG_LEVEL) << __func__ << ": pa_threaded_mainloop_get_api";
pa_mainloop_api* pa_mainloop_api = pa_threaded_mainloop_get_api(pa_mainloop);
pa_context* pa_context = pa_context_new(pa_mainloop_api, "Chrome input");
if (!pa_context) {
......@@ -233,38 +223,29 @@ bool InitPulse(pa_threaded_mainloop** mainloop, pa_context** context) {
pa_context_set_state_callback(pa_context, &SignalReadyOrErrorStateCallback,
&data);
DVLOG(DVLOG_LEVEL) << __func__ << ": pa_context_connect";
if (pa_context_connect(pa_context, nullptr, PA_CONTEXT_NOAUTOSPAWN,
nullptr)) {
DVLOG(DVLOG_LEVEL) << "Failed to connect to the context. Error: "
<< pa_strerror(pa_context_errno(pa_context));
VLOG(1) << "Failed to connect to the context. Error: "
<< pa_strerror(pa_context_errno(pa_context));
DestroyContext(pa_context);
DVLOG(DVLOG_LEVEL) << __func__ << ": pa_threaded_mainloop_free";
pa_threaded_mainloop_free(pa_mainloop);
return false;
}
// Lock the event loop object, effectively blocking the event loop thread
// from processing events. This is necessary.
DVLOG(DVLOG_LEVEL) << __func__ << ": AutoPulseLock";
auto mainloop_lock = std::make_unique<AutoPulseLock>(pa_mainloop);
// Start the threaded mainloop after everything has been configured.
DVLOG(DVLOG_LEVEL) << __func__ << ": pa_threaded_mainloop_start";
if (pa_threaded_mainloop_start(pa_mainloop)) {
DVLOG(DVLOG_LEVEL) << "Failed to start the mainloop. Error: "
<< pa_strerror(pa_context_errno(pa_context));
DestroyContext(pa_context);
DVLOG(DVLOG_LEVEL) << __func__ << ": ~AutoPulseLock";
mainloop_lock.reset();
DVLOG(DVLOG_LEVEL) << __func__ << ": pa_threaded_mainloop_free";
DestroyMainloop(pa_mainloop);
return false;
}
// Don't hold the mainloop lock while waiting for the context to become ready,
// or we'll never complete since PulseAudio can't continue working.
DVLOG(DVLOG_LEVEL) << __func__ << ": ~AutoPulseLock";
mainloop_lock.reset();
// Wait for up to 5 seconds for pa_context to become ready. We'll be signaled
......@@ -274,39 +255,30 @@ bool InitPulse(pa_threaded_mainloop** mainloop, pa_context** context) {
// browser startup (other times it's during audio process startup). In the
// normal case, this should only take ~50ms, but we've seen some test bots
// hang indefinitely when the pulse daemon can't be started.
DVLOG(DVLOG_LEVEL) << __func__ << ": Waiting for context to become ready.";
constexpr base::TimeDelta kStartupTimeout = base::TimeDelta::FromSeconds(5);
const bool was_signaled = context_wait.TimedWait(kStartupTimeout);
// Require the mainloop lock before checking the context state.
DVLOG(DVLOG_LEVEL) << __func__ << ": AutoPulseLock";
mainloop_lock = std::make_unique<AutoPulseLock>(pa_mainloop);
DVLOG(DVLOG_LEVEL) << __func__ << ": pa_context_get_state...";
auto context_state = pa_context_get_state(pa_context);
if (context_state != PA_CONTEXT_READY) {
if (!was_signaled) {
DVLOG(DVLOG_LEVEL) << "Timed out trying to connect to PulseAudio.";
} else {
DVLOG(DVLOG_LEVEL) << "Failed to connect to PulseAudio: "
<< context_state;
}
if (!was_signaled)
VLOG(1) << "Timed out trying to connect to PulseAudio.";
else
VLOG(1) << "Failed to connect to PulseAudio: " << context_state;
DestroyContext(pa_context);
DVLOG(DVLOG_LEVEL) << __func__ << ": ~AutoPulseLock";
mainloop_lock.reset();
DVLOG(DVLOG_LEVEL) << __func__ << ": pa_threaded_mainloop_free";
DestroyMainloop(pa_mainloop);
return false;
}
// Replace our function local state callback with a global appropriate one.
DVLOG(DVLOG_LEVEL) << __func__ << ": pa_context_set_state_callback";
pa_context_set_state_callback(pa_context, &pulse::ContextStateCallback,
pa_mainloop);
*mainloop = pa_mainloop;
*context = pa_context;
DVLOG(DVLOG_LEVEL) << __func__ << ": Success!";
return true;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment