Only write to mDevice once
This fixes a bug where readers might see intermediate values.
Also add comments about how mStandby and mDevice are used.
Change-Id: Idc84e56c21381a45137a2ca5ff9c57d437201869
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 5bcbaf4..d25ccfc 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -1127,8 +1127,7 @@
mChannelCount(0),
mFrameSize(1), mFormat(AUDIO_FORMAT_INVALID),
mParamStatus(NO_ERROR),
- mStandby(false), mId(id),
- mDevice(device),
+ mStandby(false), mDevice(device), mId(id),
mDeathRecipient(new PMDeathRecipient(this))
{
}
@@ -3448,7 +3447,7 @@
// forward device change to effects that have requested to be
// aware of attached audio device.
- mDevice = (uint32_t)value;
+ mDevice = value;
for (size_t i = 0; i < mEffectChains.size(); i++) {
mEffectChains[i]->setDevice_l(mDevice);
}
@@ -4036,6 +4035,7 @@
return false;
}
PlaybackThread *playbackThread = (PlaybackThread *)thread.get();
+ // see note at standby() declaration
if (playbackThread->standby() && !playbackThread->isSuspended()) {
ALOGV("DuplicatingThread output track %p on thread %p Not Ready", outputTracks[i].get(), thread.get());
return false;
@@ -6474,11 +6474,12 @@
// store input device and output device but do not forward output device to audio HAL.
// Note that status is ignored by the caller for output device
// (see AudioFlinger::setParameters()
+ audio_devices_t newDevice = mDevice;
if (value & AUDIO_DEVICE_OUT_ALL) {
- mDevice &= (uint32_t)~(value & AUDIO_DEVICE_OUT_ALL);
+ newDevice &= (uint32_t)~(value & AUDIO_DEVICE_OUT_ALL);
status = BAD_VALUE;
} else {
- mDevice &= (uint32_t)~(value & AUDIO_DEVICE_IN_ALL);
+ newDevice &= (uint32_t)~(value & AUDIO_DEVICE_IN_ALL);
// disable AEC and NS if the device is a BT SCO headset supporting those pre processings
if (mTrack != NULL) {
bool suspend = audio_is_bluetooth_sco_device(
@@ -6487,7 +6488,8 @@
setEffectSuspended_l(FX_IID_NS, suspend, mTrack->sessionId());
}
}
- mDevice |= (uint32_t)value;
+ newDevice |= value;
+ mDevice = newDevice; // since mDevice is read by other threads, only write to it once
}
if (status == NO_ERROR) {
status = mInput->stream->common.set_parameters(&mInput->stream->common, keyValuePair.string());
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 685764f..b8bb69e 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -484,13 +484,19 @@
};
virtual status_t initCheck() const = 0;
+
+ // static externally-visible
type_t type() const { return mType; }
+ audio_io_handle_t id() const { return mId;}
+
+ // dynamic externally-visible
uint32_t sampleRate() const { return mSampleRate; }
int channelCount() const { return mChannelCount; }
audio_format_t format() const { return mFormat; }
// Called by AudioFlinger::frameCount(audio_io_handle_t output) and effects,
// and returns the normal mix buffer's frame count. No API for HAL frame count.
size_t frameCount() const { return mNormalFrameCount; }
+
void wakeUp() { mWaitWorkCV.broadcast(); }
// Should be "virtual status_t requestExitAndWait()" and override same
// method in Thread, but Thread::requestExitAndWait() is not yet virtual.
@@ -502,9 +508,11 @@
void sendConfigEvent(int event, int param = 0);
void sendConfigEvent_l(int event, int param = 0);
void processConfigEvents();
- audio_io_handle_t id() const { return mId;}
+
+ // see note at declaration of mStandby and mDevice
bool standby() const { return mStandby; }
- uint32_t device() const { return mDevice; }
+ audio_devices_t device() const { return mDevice; }
+
virtual audio_stream_t* stream() const = 0;
sp<EffectHandle> createEffect_l(
@@ -647,11 +655,19 @@
status_t mParamStatus;
Vector<ConfigEvent> mConfigEvents;
- bool mStandby;
+
+ // These fields are written and read by thread itself without lock or barrier,
+ // and read by other threads without lock or barrier via standby() and device().
+ // Because of the absence of a lock or barrier, any other thread that reads
+ // these fields must use the information in isolation, or be prepared to deal
+ // with possibility that it might be inconsistent with other information.
+ bool mStandby; // Whether thread is currently in standby.
+ audio_devices_t mDevice; // output device for PlaybackThread
+ // input + output devices for RecordThread
+
const audio_io_handle_t mId;
Vector< sp<EffectChain> > mEffectChains;
- uint32_t mDevice; // output device for PlaybackThread
- // input + output devices for RecordThread
+
static const int kNameLength = 16; // prctl(PR_SET_NAME) limit
char mName[kNameLength];
sp<IPowerManager> mPowerManager;