Cherry pick PR #7110: starboard: Check pthread_join completes successfully (#7120)

cobalt-github-releaser-bot · kjyoun · web-flow · commit 6341c9739774 · 2025-09-09T18:38:08.000-07:00
Refer to the original PR: #7110 The code relies on a fundamental invariant: after pthread_join() returns, the joined thread is guaranteed to have terminated. This means no more callbacks will be made from that thread. A failure in `pthread_join()` breaches this invariant and leads to an unexpected state where the supposedly-completed thread is still running and calling callbacks. This can lead to use-after-free errors or other race conditions. This change adds a check to enforce this invariant, preventing the application from continuing in a corrupt state. #vibe-coded Bug: 443334872 --------- Co-authored-by: Kyujung Youn <kjyoun@google.com>
diff --git a/starboard/android/shared/audio_track_audio_sink_type.cc b/starboard/android/shared/audio_track_audio_sink_type.cc
@@ -173,7 +173,7 @@ AudioTrackAudioSink::~AudioTrackAudioSink() {
   quit_ = true;
 
   if (audio_out_thread_ != 0) {
-    pthread_join(audio_out_thread_, NULL);
+    SB_CHECK_EQ(pthread_join(audio_out_thread_, nullptr), 0);
   }
 }
 
diff --git a/starboard/android/shared/continuous_audio_track_sink.cc b/starboard/android/shared/continuous_audio_track_sink.cc
@@ -103,7 +103,7 @@ ContinuousAudioTrackSink::~ContinuousAudioTrackSink() {
   quit_ = true;
 
   if (audio_out_thread_ != 0) {
-    pthread_join(audio_out_thread_, NULL);
+    SB_CHECK_EQ(pthread_join(audio_out_thread_, nullptr), 0);
   }
 }
 
diff --git a/starboard/android/shared/media_decoder.cc b/starboard/android/shared/media_decoder.cc
@@ -394,7 +394,7 @@ void MediaDecoder::TerminateDecoderThread() {
   }
 
   if (decoder_thread_ != 0) {
-    pthread_join(decoder_thread_, nullptr);
+    SB_CHECK_EQ(pthread_join(decoder_thread_, nullptr), 0);
     decoder_thread_ = 0;
   }
 }
diff --git a/starboard/shared/starboard/audio_sink/stub_audio_sink_type.cc b/starboard/shared/starboard/audio_sink/stub_audio_sink_type.cc
@@ -82,7 +82,7 @@ StubAudioSink::~StubAudioSink() {
     ScopedLock lock(mutex_);
     destroying_ = true;
   }
-  pthread_join(audio_out_thread_, NULL);
+  SB_CHECK_EQ(pthread_join(audio_out_thread_, nullptr), 0);
 }
 
 // static
diff --git a/starboard/shared/starboard/player/filter/punchout_video_renderer_sink.cc b/starboard/shared/starboard/player/filter/punchout_video_renderer_sink.cc
@@ -42,7 +42,7 @@ PunchoutVideoRendererSink::PunchoutVideoRendererSink(SbPlayer player,
 PunchoutVideoRendererSink::~PunchoutVideoRendererSink() {
   if (thread_ != 0) {
     stop_requested_.store(true);
-    pthread_join(thread_, NULL);
+    SB_CHECK_EQ(pthread_join(thread_, nullptr), 0);
   }
 }
 
diff --git a/starboard/shared/starboard/player/player_worker.cc b/starboard/shared/starboard/player/player_worker.cc
@@ -91,7 +91,7 @@ PlayerWorker::~PlayerWorker() {
 
   if (thread_ != 0) {
     job_queue_->Schedule(std::bind(&PlayerWorker::DoStop, this));
-    pthread_join(thread_, NULL);
+    SB_CHECK_EQ(pthread_join(thread_, nullptr), 0);
     thread_ = 0;
 
     // Now the whole pipeline has been torn down and no callback will be called.

Original file line number	Diff line number	Diff line change
`@@ -173,7 +173,7 @@ AudioTrackAudioSink::~AudioTrackAudioSink() {`
`173`	`173`	`quit_ = true;`
`174`	`174`
`175`	`175`	`if (audio_out_thread_ != 0) {`
`176`		`- pthread_join(audio_out_thread_, NULL);`
	`176`	`+ SB_CHECK_EQ(pthread_join(audio_out_thread_, nullptr), 0);`
`177`	`177`	`}`
`178`	`178`	`}`
`179`	`179`
Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ ContinuousAudioTrackSink::~ContinuousAudioTrackSink() {`
`103`	`103`	`quit_ = true;`
`104`	`104`
`105`	`105`	`if (audio_out_thread_ != 0) {`
`106`		`- pthread_join(audio_out_thread_, NULL);`
	`106`	`+ SB_CHECK_EQ(pthread_join(audio_out_thread_, nullptr), 0);`
`107`	`107`	`}`
`108`	`108`	`}`
`109`	`109`
Original file line number	Diff line number	Diff line change
`@@ -394,7 +394,7 @@ void MediaDecoder::TerminateDecoderThread() {`
`394`	`394`	`}`
`395`	`395`
`396`	`396`	`if (decoder_thread_ != 0) {`
`397`		`- pthread_join(decoder_thread_, nullptr);`
	`397`	`+ SB_CHECK_EQ(pthread_join(decoder_thread_, nullptr), 0);`
`398`	`398`	`decoder_thread_ = 0;`
`399`	`399`	`}`
`400`	`400`	`}`
Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ StubAudioSink::~StubAudioSink() {`
`82`	`82`	`ScopedLock lock(mutex_);`
`83`	`83`	`destroying_ = true;`
`84`	`84`	`}`
`85`		`- pthread_join(audio_out_thread_, NULL);`
	`85`	`+ SB_CHECK_EQ(pthread_join(audio_out_thread_, nullptr), 0);`
`86`	`86`	`}`
`87`	`87`
`88`	`88`	`// static`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ PunchoutVideoRendererSink::PunchoutVideoRendererSink(SbPlayer player,`
`42`	`42`	`PunchoutVideoRendererSink::~PunchoutVideoRendererSink() {`
`43`	`43`	`if (thread_ != 0) {`
`44`	`44`	`stop_requested_.store(true);`
`45`		`- pthread_join(thread_, NULL);`
	`45`	`+ SB_CHECK_EQ(pthread_join(thread_, nullptr), 0);`
`46`	`46`	`}`
`47`	`47`	`}`
`48`	`48`