Skip to content

Commit e57a811

Browse files
kjyounkjyoun
authored
starboard: Enforce thread safety invariants in release builds (#7112)
The AudioRendererPcm/MediaDecoder and their underlying sink components are designed with a critical invariant: they are not thread-safe and their methods must be called from a single, consistent thread. Previously, this invariant was only verified by SB_DCHECK, which is compiled out of release builds. Because this check is compiled out of release builds, it was unable to detect race conditions occurring in the production environment. It make debugging from prod crash report very difficult. This change promotes these thread-safety checks to SB_CHECK. Now, if any method is called from an incorrect thread, the application will crash immediately with a clear stack trace at the site of the violation. This prevents the system from entering a corrupt state and makes diagnosing the root cause of such threading bugs significantly more straightforward. #vibe-coded Bug: 443334872 Bug: 435425692
1 parent 9c4102d commit e57a811

File tree

4 files changed

+35
-35
lines changed

4 files changed

+35
-35
lines changed

‎starboard/android/shared/media_decoder.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ MediaDecoder::MediaDecoder(
149149
}
150150

151151
MediaDecoder::~MediaDecoder() {
152-
SB_DCHECK(thread_checker_.CalledOnValidThread());
152+
SB_CHECK(thread_checker_.CalledOnValidThread());
153153

154154
TerminateDecoderThread();
155155

@@ -170,7 +170,7 @@ MediaDecoder::~MediaDecoder() {
170170
}
171171

172172
void MediaDecoder::Initialize(const ErrorCB& error_cb) {
173-
SB_DCHECK(thread_checker_.CalledOnValidThread());
173+
SB_CHECK(thread_checker_.CalledOnValidThread());
174174
SB_DCHECK(error_cb);
175175
SB_DCHECK(!error_cb_);
176176

@@ -182,7 +182,7 @@ void MediaDecoder::Initialize(const ErrorCB& error_cb) {
182182
}
183183

184184
void MediaDecoder::WriteInputBuffers(const InputBuffers& input_buffers) {
185-
SB_DCHECK(thread_checker_.CalledOnValidThread());
185+
SB_CHECK(thread_checker_.CalledOnValidThread());
186186
if (stream_ended_.load()) {
187187
SB_LOG(ERROR) << "Decode() is called after WriteEndOfStream() is called.";
188188
return;
@@ -213,7 +213,7 @@ void MediaDecoder::WriteInputBuffers(const InputBuffers& input_buffers) {
213213
}
214214

215215
void MediaDecoder::WriteEndOfStream() {
216-
SB_DCHECK(thread_checker_.CalledOnValidThread());
216+
SB_CHECK(thread_checker_.CalledOnValidThread());
217217

218218
stream_ended_.store(true);
219219
std::lock_guard lock(mutex_);
@@ -388,7 +388,7 @@ void MediaDecoder::DecoderThreadFunc() {
388388
}
389389

390390
void MediaDecoder::TerminateDecoderThread() {
391-
SB_DCHECK(thread_checker_.CalledOnValidThread());
391+
SB_CHECK(thread_checker_.CalledOnValidThread());
392392

393393
destroying_.store(true);
394394

@@ -705,7 +705,7 @@ void MediaDecoder::OnMediaCodecFirstTunnelFrameReady() {
705705
}
706706

707707
bool MediaDecoder::Flush() {
708-
SB_DCHECK(thread_checker_.CalledOnValidThread());
708+
SB_CHECK(thread_checker_.CalledOnValidThread());
709709

710710
// Try to flush if we can, otherwise return |false| to recreate the codec
711711
// completely. Flush() is called by `player_worker` thread,

‎starboard/android/shared/video_decoder.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ VideoDecoder::GetRenderAlgorithm() {
458458

459459
void VideoDecoder::Initialize(const DecoderStatusCB& decoder_status_cb,
460460
const ErrorCB& error_cb) {
461-
SB_DCHECK(BelongsToCurrentThread());
461+
SB_CHECK(BelongsToCurrentThread());
462462
SB_DCHECK(decoder_status_cb);
463463
SB_DCHECK(!decoder_status_cb_);
464464
SB_DCHECK(error_cb);
@@ -497,7 +497,7 @@ int64_t VideoDecoder::GetPrerollTimeout() const {
497497
}
498498

499499
void VideoDecoder::WriteInputBuffers(const InputBuffers& input_buffers) {
500-
SB_DCHECK(BelongsToCurrentThread());
500+
SB_CHECK(BelongsToCurrentThread());
501501
SB_DCHECK(!input_buffers.empty());
502502
SB_DCHECK_EQ(input_buffers.front()->sample_type(), kSbMediaTypeVideo);
503503
SB_DCHECK(decoder_status_cb_);
@@ -567,7 +567,7 @@ void VideoDecoder::WriteInputBuffers(const InputBuffers& input_buffers) {
567567
}
568568

569569
void VideoDecoder::WriteEndOfStream() {
570-
SB_DCHECK(BelongsToCurrentThread());
570+
SB_CHECK(BelongsToCurrentThread());
571571
SB_DCHECK(decoder_status_cb_);
572572

573573
if (end_of_stream_written_) {
@@ -616,7 +616,7 @@ void VideoDecoder::WriteEndOfStream() {
616616
}
617617

618618
void VideoDecoder::Reset() {
619-
SB_DCHECK(BelongsToCurrentThread());
619+
SB_CHECK(BelongsToCurrentThread());
620620

621621
// If fail to flush |media_decoder_| or |media_decoder_| is null, then
622622
// re-create |media_decoder_|. If the codec is kSbMediaVideoCodecAv1,
@@ -653,7 +653,7 @@ void VideoDecoder::Reset() {
653653

654654
bool VideoDecoder::InitializeCodec(const VideoStreamInfo& video_stream_info,
655655
std::string* error_message) {
656-
SB_DCHECK(BelongsToCurrentThread());
656+
SB_CHECK(BelongsToCurrentThread());
657657
SB_CHECK(error_message);
658658

659659
if (video_stream_info.codec == kSbMediaVideoCodecAv1) {
@@ -776,7 +776,7 @@ bool VideoDecoder::InitializeCodec(const VideoStreamInfo& video_stream_info,
776776
}
777777

778778
void VideoDecoder::TeardownCodec() {
779-
SB_DCHECK(BelongsToCurrentThread());
779+
SB_CHECK(BelongsToCurrentThread());
780780
if (owns_video_surface_) {
781781
ReleaseVideoSurface();
782782
owns_video_surface_ = false;
@@ -1211,14 +1211,14 @@ void VideoDecoder::OnFirstTunnelFrameReady() {
12111211
}
12121212

12131213
void VideoDecoder::OnTunnelModePrerollTimeout() {
1214-
SB_DCHECK(BelongsToCurrentThread());
1214+
SB_CHECK(BelongsToCurrentThread());
12151215
SB_DCHECK_NE(tunnel_mode_audio_session_id_, -1);
12161216

12171217
TryToSignalPrerollForTunnelMode();
12181218
}
12191219

12201220
void VideoDecoder::OnTunnelModeCheckForNeedMoreInput() {
1221-
SB_DCHECK(BelongsToCurrentThread());
1221+
SB_CHECK(BelongsToCurrentThread());
12221222
SB_DCHECK_NE(tunnel_mode_audio_session_id_, -1);
12231223

12241224
// There's a race condition when suspending the app. If surface view is

‎starboard/shared/starboard/player/filter/audio_renderer_internal_pcm.cc

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ AudioRendererPcm::~AudioRendererPcm() {
102102
<< " channels, " << bytes_per_frame_ << " bytes per frame, "
103103
<< max_cached_frames_ << " max cached frames, and "
104104
<< min_frames_per_append_ << " min frames per append.";
105-
SB_DCHECK(BelongsToCurrentThread());
105+
SB_CHECK(BelongsToCurrentThread());
106106
}
107107

108108
void AudioRendererPcm::Initialize(const ErrorCB& error_cb,
@@ -124,7 +124,7 @@ void AudioRendererPcm::Initialize(const ErrorCB& error_cb,
124124
}
125125

126126
void AudioRendererPcm::WriteSamples(const InputBuffers& input_buffers) {
127-
SB_DCHECK(BelongsToCurrentThread());
127+
SB_CHECK(BelongsToCurrentThread());
128128
SB_DCHECK(!input_buffers.empty());
129129
SB_DCHECK(can_accept_more_data_);
130130

@@ -142,7 +142,7 @@ void AudioRendererPcm::WriteSamples(const InputBuffers& input_buffers) {
142142
}
143143

144144
void AudioRendererPcm::WriteEndOfStream() {
145-
SB_DCHECK(BelongsToCurrentThread());
145+
SB_CHECK(BelongsToCurrentThread());
146146
// TODO: Check |can_accept_more_data_| and make WriteEndOfStream() depend on
147147
// CanAcceptMoreData() or callback.
148148
// SB_DCHECK(can_accept_more_data_);
@@ -161,12 +161,12 @@ void AudioRendererPcm::WriteEndOfStream() {
161161
}
162162

163163
void AudioRendererPcm::SetVolume(double volume) {
164-
SB_DCHECK(BelongsToCurrentThread());
164+
SB_CHECK(BelongsToCurrentThread());
165165
audio_renderer_sink_->SetVolume(volume);
166166
}
167167

168168
bool AudioRendererPcm::IsEndOfStreamWritten() const {
169-
SB_DCHECK(BelongsToCurrentThread());
169+
SB_CHECK(BelongsToCurrentThread());
170170
return eos_state_ >= kEOSWrittenToDecoder;
171171
}
172172

@@ -176,28 +176,28 @@ bool AudioRendererPcm::IsEndOfStreamPlayed() const {
176176
}
177177

178178
bool AudioRendererPcm::CanAcceptMoreData() const {
179-
SB_DCHECK(BelongsToCurrentThread());
179+
SB_CHECK(BelongsToCurrentThread());
180180
return eos_state_ == kEOSNotReceived && can_accept_more_data_ &&
181181
(!decoder_sample_rate_ || !time_stretcher_.IsQueueFull());
182182
}
183183

184184
void AudioRendererPcm::Play() {
185-
SB_DCHECK(BelongsToCurrentThread());
185+
SB_CHECK(BelongsToCurrentThread());
186186

187187
std::lock_guard lock(mutex_);
188188
paused_ = false;
189189
consume_frames_called_ = false;
190190
}
191191

192192
void AudioRendererPcm::Pause() {
193-
SB_DCHECK(BelongsToCurrentThread());
193+
SB_CHECK(BelongsToCurrentThread());
194194

195195
std::lock_guard lock(mutex_);
196196
paused_ = true;
197197
}
198198

199199
void AudioRendererPcm::SetPlaybackRate(double playback_rate) {
200-
SB_DCHECK(BelongsToCurrentThread());
200+
SB_CHECK(BelongsToCurrentThread());
201201

202202
std::lock_guard lock(mutex_);
203203

@@ -222,7 +222,7 @@ void AudioRendererPcm::SetPlaybackRate(double playback_rate) {
222222
}
223223

224224
void AudioRendererPcm::Seek(int64_t seek_to_time) {
225-
SB_DCHECK(BelongsToCurrentThread());
225+
SB_CHECK(BelongsToCurrentThread());
226226
SB_DCHECK_GE(seek_to_time, 0);
227227

228228
audio_renderer_sink_->Stop();
@@ -499,7 +499,7 @@ void AudioRendererPcm::OnFirstOutput(
499499
const SbMediaAudioSampleType decoded_sample_type,
500500
const SbMediaAudioFrameStorageType decoded_storage_type,
501501
const int decoded_sample_rate) {
502-
SB_DCHECK(BelongsToCurrentThread());
502+
SB_CHECK(BelongsToCurrentThread());
503503
SB_DCHECK(!decoder_sample_rate_);
504504
decoder_sample_rate_ = decoded_sample_rate;
505505
int destination_sample_rate =
@@ -541,7 +541,7 @@ bool AudioRendererPcm::IsEndOfStreamPlayed_Locked() const {
541541
}
542542

543543
void AudioRendererPcm::OnDecoderConsumed() {
544-
SB_DCHECK(BelongsToCurrentThread());
544+
SB_CHECK(BelongsToCurrentThread());
545545

546546
// TODO: Unify EOS and non EOS request once WriteEndOfStream() depends on
547547
// CanAcceptMoreData().
@@ -553,7 +553,7 @@ void AudioRendererPcm::OnDecoderConsumed() {
553553
}
554554

555555
void AudioRendererPcm::OnDecoderOutput() {
556-
SB_DCHECK(BelongsToCurrentThread());
556+
SB_CHECK(BelongsToCurrentThread());
557557

558558
++pending_decoder_outputs_;
559559

@@ -566,7 +566,7 @@ void AudioRendererPcm::OnDecoderOutput() {
566566
}
567567

568568
void AudioRendererPcm::ProcessAudioData() {
569-
SB_DCHECK(BelongsToCurrentThread());
569+
SB_CHECK(BelongsToCurrentThread());
570570

571571
process_audio_data_job_token_.ResetToInvalid();
572572

@@ -668,7 +668,7 @@ void AudioRendererPcm::ProcessAudioData() {
668668
}
669669

670670
bool AudioRendererPcm::AppendAudioToFrameBuffer(bool* is_frame_buffer_full) {
671-
SB_DCHECK(BelongsToCurrentThread());
671+
SB_CHECK(BelongsToCurrentThread());
672672
SB_DCHECK(is_frame_buffer_full);
673673

674674
*is_frame_buffer_full = false;
@@ -738,7 +738,7 @@ bool AudioRendererPcm::AppendAudioToFrameBuffer(bool* is_frame_buffer_full) {
738738

739739
#if SB_PLAYER_FILTER_ENABLE_STATE_CHECK
740740
void AudioRendererPcm::CheckAudioSinkStatus() {
741-
SB_DCHECK(BelongsToCurrentThread());
741+
SB_CHECK(BelongsToCurrentThread());
742742

743743
// Check if sink callbacks are called too frequently.
744744
if (sink_callbacks_since_last_check_.load() > kMaxSinkCallbacksBetweenCheck) {

‎starboard/shared/starboard/player/filter/audio_renderer_sink_impl.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ AudioRendererSinkImpl::AudioRendererSinkImpl(
5050
}
5151

5252
AudioRendererSinkImpl::~AudioRendererSinkImpl() {
53-
SB_DCHECK(thread_checker_.CalledOnValidThread());
53+
SB_CHECK(thread_checker_.CalledOnValidThread());
5454

5555
Stop();
5656
}
@@ -83,7 +83,7 @@ void AudioRendererSinkImpl::Start(
8383
SbAudioSinkFrameBuffers frame_buffers,
8484
int frames_per_channel,
8585
RenderCallback* render_callback) {
86-
SB_DCHECK(thread_checker_.CalledOnValidThread());
86+
SB_CHECK(thread_checker_.CalledOnValidThread());
8787
SB_DCHECK(!HasStarted());
8888
SB_DCHECK(channels > 0 && channels <= SbAudioSinkGetMaxChannels());
8989
SB_DCHECK_GT(sampling_frequency_hz, 0);
@@ -111,7 +111,7 @@ void AudioRendererSinkImpl::Start(
111111
}
112112

113113
void AudioRendererSinkImpl::Stop() {
114-
SB_DCHECK(thread_checker_.CalledOnValidThread());
114+
SB_CHECK(thread_checker_.CalledOnValidThread());
115115

116116
if (HasStarted()) {
117117
SbAudioSinkDestroy(audio_sink_);
@@ -121,7 +121,7 @@ void AudioRendererSinkImpl::Stop() {
121121
}
122122

123123
void AudioRendererSinkImpl::SetVolume(double volume) {
124-
SB_DCHECK(thread_checker_.CalledOnValidThread());
124+
SB_CHECK(thread_checker_.CalledOnValidThread());
125125

126126
volume_ = volume;
127127
if (HasStarted()) {
@@ -130,7 +130,7 @@ void AudioRendererSinkImpl::SetVolume(double volume) {
130130
}
131131

132132
void AudioRendererSinkImpl::SetPlaybackRate(double playback_rate) {
133-
SB_DCHECK(thread_checker_.CalledOnValidThread());
133+
SB_CHECK(thread_checker_.CalledOnValidThread());
134134
SB_DCHECK(playback_rate == 0.0 || playback_rate == 1.0)
135135
<< "Playback rate on audio sink can only be set to 0 or 1, "
136136
<< "but is now set to " << playback_rate;

0 commit comments

Comments
 (0)