From cdbd12e2b3fca546c032b40d955dce821712a0d1 Mon Sep 17 00:00:00 2001
From: Johannes Kron <[email protected]>
Date: Wed, 01 Apr 2026 14:51:11 -0700
Subject: [PATCH] Synchronize IoProc lifecycle with a proxy object

Introduce CatapIoProcProxy to manage synchronization between the
CoreAudio IOProc and CatapAudioInputStreamSource. This ensures that
callbacks from the OS audio thread are handled safely during and after
stream teardown.

The proxy uses a lock and a boolean flag to synchronously fence off
callbacks when the source is stopping. If the OS fails to synchronously
terminate the IO process during Close(), the proxy is intentionally.
leaked to provide the orphaned thread with a valid memory address.

Bug: 495779613
Change-Id: Ied533d2625664b4bb78a1505152a337339c8d52c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7708301
Reviewed-by: Olga Sharonova <[email protected]>
Commit-Queue: Johannes Kron <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1608788}
---

diff --git a/media/audio/mac/catap_audio_input_stream.h b/media/audio/mac/catap_audio_input_stream.h
index 3e714c4a..3ece2e1 100644
--- a/media/audio/mac/catap_audio_input_stream.h
+++ b/media/audio/mac/catap_audio_input_stream.h
@@ -27,6 +27,8 @@
 namespace media {
 
 class CatapApi;
+class CatapIoProcProxy;
+
 class PropertyListenerHelper;
 
 // Captures audio loopback using the CoreAudio API for macOS 14.2
@@ -238,6 +240,13 @@
       kAudioObjectUnknown;
   CATapDescription* __strong tap_description_
       GUARDED_BY_CONTEXT(sequence_checker_) = nil;
+  // Tracks if the synchronous fences failed during teardown.
+  bool stop_failed_ = false;
+
+  // The proxy passed to CoreAudio.
+  std::unique_ptr<CatapIoProcProxy> io_proc_proxy_
+      GUARDED_BY_CONTEXT(sequence_checker_);
+
   bool is_device_open_ GUARDED_BY_CONTEXT(sequence_checker_) = false;
 
   SEQUENCE_CHECKER(sequence_checker_);
diff --git a/media/audio/mac/catap_audio_input_stream.mm b/media/audio/mac/catap_audio_input_stream.mm
index 3ec0028..3de2da0 100644
--- a/media/audio/mac/catap_audio_input_stream.mm
+++ b/media/audio/mac/catap_audio_input_stream.mm
@@ -14,15 +14,18 @@
 
 #include <string_view>
 
+#include "base/debug/leak_annotations.h"
 #include "base/feature_list.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback.h"
 #include "base/logging.h"
+#include "base/memory/raw_ptr_exclusion.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/process/process.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
+#include "base/synchronization/lock.h"
 #include "base/timer/elapsed_timer.h"
 #include "base/trace_event/trace_event.h"
 #include "media/audio/application_loopback_device_helper.h"
@@ -35,6 +38,36 @@
 #include "media/base/audio_timestamp_helper.h"
 
 namespace media {
+// Acts as a thread-safe bridge between the CoreAudio IOProc and the
+// CatapAudioInputStreamSource. If teardown fails, this object is intentionally
+// leaked to give the orphaned OS thread a valid memory address to read.
+class API_AVAILABLE(macos(14.2)) CatapIoProcProxy {
+ public:
+  CatapIoProcProxy(raw_ptr<CatapAudioInputStreamSource> source)
+      : source_(source) {}
+
+  // Called from the main sequence during teardown.
+  void Detach() {
+    base::AutoLock auto_lock(lock_);
+    source_ = nullptr;
+  }
+
+  // Called by the CoreAudio high-priority thread.
+  void ForwardSample(const AudioBuffer* input_buffer,
+                     const AudioTimeStamp* input_time) {
+    base::AutoLock auto_lock(lock_);
+    if (source_) {
+      source_->OnCatapSample(input_buffer, input_time);
+    }
+  }
+
+ private:
+  // Lock to protect access to source_ and to ensure that ForwardSample()
+  // finishes before Detach() returns.
+  base::Lock lock_;
+  raw_ptr<CatapAudioInputStreamSource> source_ GUARDED_BY(lock_);
+};
+
 namespace {
 const char kCatapAudioInputStreamUmaBaseName[] =
     "Media.Audio.Mac.CatapAudioInputStream";
@@ -115,9 +148,8 @@
                       AudioBufferList* output_data,
                       const AudioTimeStamp* output_time,
                       void* client_data) {
-  CatapAudioInputStreamSource* catap_input_stream =
-      reinterpret_cast<CatapAudioInputStreamSource*>(client_data);
-  CHECK(catap_input_stream != nullptr);
+  CatapIoProcProxy* proxy = reinterpret_cast<CatapIoProcProxy*>(client_data);
+  CHECK(proxy != nullptr);
 
   // Multiple buffers correspond to multiple streams. This is not expected
   // during system audio capture, and the OnCatapSample() function is designed
@@ -127,7 +159,7 @@
   DCHECK_EQ(input_data->mNumberBuffers, 1u);
 
   if (input_data->mNumberBuffers > 0 && input_data->mBuffers->mData != NULL) {
-    catap_input_stream->OnCatapSample(input_data->mBuffers, input_time);
+    proxy->ForwardSample(input_data->mBuffers, input_time);
   }
   return noErr;
 }
@@ -649,11 +681,13 @@
   // dialog. If the user doesn't respond to the dialog, this call will time out
   // in 60 seconds. When this happens all interactions with CoreAudio will fail
   // until the audio process is restarted.
+  io_proc_proxy_ = std::make_unique<CatapIoProcProxy>(this);
   {
     constexpr base::TimeDelta kCreateIoProcIdTimeout = base::Seconds(59);
     base::ElapsedTimer create_io_proc_id_timer;
     status = catap_api_->AudioDeviceCreateIOProcID(
-        aggregate_device_id_, DeviceIoProc, this, &tap_io_proc_id_);
+        aggregate_device_id_, DeviceIoProc, io_proc_proxy_.get(),
+        &tap_io_proc_id_);
     if (base::FeatureList::IsEnabled(
             features::kMacCatapRestartAudioProcessOnTimeout) &&
         create_io_proc_id_timer.Elapsed() > kCreateIoProcIdTimeout) {
@@ -723,6 +757,13 @@
   SendLogMessage("%s", __func__);
   base::ElapsedTimer timer;
 
+  // Instantly fence off the CoreAudio thread.
+  // If the OS thread is currently in the callback, this blocks until it
+  // finishes.
+  if (io_proc_proxy_) {
+    io_proc_proxy_->Detach();
+  }
+
   property_listener_.reset();
 
   if (!sink_) {
@@ -733,12 +774,15 @@
   CHECK_NE(tap_io_proc_id_, nullptr);
 
   // Reversing Step 4.
-  // The call to AudioDeviceStop is synchronous. It will not return until any
-  // current callbacks have finished executing. The call to AudioDeviceStop()
-  // succeeds even though AudioDeviceStart() has not been called.
+  // AudioDeviceStop is synchronous when it succeeds, but may not be if it
+  // fails. The lock above mitigates the failure case by acting as a synchronous
+  // fence, ensuring that no callbacks are actively executing before we proceed.
+  // Note: The call to AudioDeviceStop() will succeed even if AudioDeviceStart()
+  // has not been called.
   OSStatus status =
       catap_api_->AudioDeviceStop(aggregate_device_id_, tap_io_proc_id_);
   if (status != noErr) {
+    stop_failed_ = true;
     ReportStopStatus(false, timer.Elapsed());
     SendLogMessage("%s => Error stopping the device. Status: %d", __func__,
                    status);
@@ -771,6 +815,7 @@
   base::ElapsedTimer timer;
 
   is_device_open_ = false;
+  bool destroy_failed = false;
 
   if (aggregate_device_id_ != kAudioObjectUnknown &&
       tap_io_proc_id_ != nullptr) {
@@ -778,6 +823,7 @@
     OSStatus status = catap_api_->AudioDeviceDestroyIOProcID(
         aggregate_device_id_, tap_io_proc_id_);
     if (status != noErr) {
+      destroy_failed = true;
       ReportCloseStatus(CloseStatus::kErrorDestroyingIOProcID, timer.Elapsed());
       SendLogMessage("%s => Error destroying device IO process ID. Status: %d",
                      __func__, status);
@@ -814,6 +860,20 @@
     tap_description_ = nil;
   }
 
+  if (io_proc_proxy_) {

diff --git a/media/audio/mac/catap_audio_input_stream_unittest.mm b/media/audio/mac/catap_audio_input_stream_unittest.mm
index 9eea760..3267baf 100644
--- a/media/audio/mac/catap_audio_input_stream_unittest.mm
+++ b/media/audio/mac/catap_audio_input_stream_unittest.mm
@@ -294,12 +294,18 @@
   }
   OSStatus AudioDeviceStop(AudioDeviceID in_device,
                            AudioDeviceIOProcID in_proc_id) override {
+    if (should_fail_audio_device_stop) {
+      return -1;
+    }
     stopped_device = in_device;
     stopped_proc_id = in_proc_id;
     return noErr;
   }
   OSStatus AudioDeviceDestroyIOProcID(AudioDeviceID in_device,
                                       AudioDeviceIOProcID in_proc_id) override {
+    if (should_fail_audio_device_destroy) {
+      return -1;
+    }
     destroyed_io_proc_id_for_device = in_device;
     destroyed_io_proc_id = in_proc_id;
     client_data = nullptr;
@@ -341,6 +347,11 @@
 
   // Public properties that can be modified by the tests.
 
+  // Controls the success of the AudioDeviceStop() call.
+  bool should_fail_audio_device_stop = false;
+  // Controls the success of the AudioDeviceDestroyIOProcID() call.
+  bool should_fail_audio_device_destroy = false;
+
   // If `true`, `AudioObjectSetPropertyData()` will return `noErr` when setting
   // the tap description. Otherwise it will return an error, which simulates
   // that the user has not given screen capture permissions.
@@ -1265,4 +1276,93 @@
   }
 }
 
+TEST_F(CatapAudioInputStreamTest, SurvivesLateCallbackIfStopFails) {
+  if (@available(macOS 14.2, *)) {
+    CreateStream();
+    EXPECT_EQ(stream_->Open(), AudioInputStream::OpenOutcome::kSuccess);
+    stream_->Start(&fake_callback_);
+
+    // Extract the OS callback and the proxy pointer (client_data) before
+    // teardown.
+    AudioDeviceIOProc audio_proc = fake_catap_api()->audio_proc;
+    void* proxy_client_data = fake_catap_api()->client_data;
+    ASSERT_NE(audio_proc, nullptr);
+    ASSERT_NE(proxy_client_data, nullptr);
+
+    // Force AudioDeviceStop to fail.
+    fake_catap_api()->should_fail_audio_device_stop = true;
+
+    // Stop and destroy the stream.
+    // Because stop fails, the proxy will be intentionally leaked.
+    stream_->Stop();
+    stream_->Close();
+    fake_catap_api_ = nullptr;
+    stream_.ClearAndDelete();
+
+    // Simulate the rogue callback.
+    const AudioTimeStamp* in_now = nullptr;
+    const uint32_t data_byte_size = kCatapLoopbackDefaultFramesPerBuffer *
+                                    sizeof(Float32) * kNumberOfChannelsStereo;
+    std::vector<uint8_t> data_buffer(data_byte_size);
+
+    AudioBufferList input_data;
+    input_data.mNumberBuffers = 1;
+    input_data.mBuffers[0].mNumberChannels = kNumberOfChannelsStereo;
+    input_data.mBuffers[0].mDataByteSize = data_byte_size;
+    input_data.mBuffers[0].mData = data_buffer.data();
+    AudioTimeStamp input_time = {};
+    AudioBufferList* output_data = nullptr;
+    const AudioTimeStamp* output_time = nullptr;
+
+    OSStatus status = audio_proc(0, in_now, &input_data, &input_time,
+                                 output_data, output_time, proxy_client_data);
+
+    EXPECT_EQ(status, noErr);
+  }
+}
+
+TEST_F(CatapAudioInputStreamTest, SurvivesLateCallbackIfDestroyFails) {
+  if (@available(macOS 14.2, *)) {
+    CreateStream();
+    EXPECT_EQ(stream_->Open(), AudioInputStream::OpenOutcome::kSuccess);
+    stream_->Start(&fake_callback_);
+
+    AudioDeviceIOProc audio_proc = fake_catap_api()->audio_proc;
+    void* proxy_client_data = fake_catap_api()->client_data;
+    ASSERT_NE(audio_proc, nullptr);
+    ASSERT_NE(proxy_client_data, nullptr);
+
+    // Force AudioDeviceDestroyIOProcID to fail.
+    fake_catap_api()->should_fail_audio_device_destroy = true;
+
+    // Stop and destroy the stream.
+    // Because destroy process IO proc fails, the proxy will be intentionally
+    // leaked.
+    stream_->Stop();
+    stream_->Close();
+    fake_catap_api_ = nullptr;
+    stream_.ClearAndDelete();
+
+    // Simulate the rogue callback.
+    const AudioTimeStamp* in_now = nullptr;
+    const uint32_t data_byte_size = kCatapLoopbackDefaultFramesPerBuffer *
+                                    sizeof(Float32) * kNumberOfChannelsStereo;
+    std::vector<uint8_t> data_buffer(data_byte_size);
+
+    AudioBufferList input_data;
+    input_data.mNumberBuffers = 1;
+    input_data.mBuffers[0].mNumberChannels = kNumberOfChannelsStereo;
+    input_data.mBuffers[0].mDataByteSize = data_byte_size;
+    input_data.mBuffers[0].mData = data_buffer.data();
+    AudioTimeStamp input_time = {};
+    AudioBufferList* output_data = nullptr;
+    const AudioTimeStamp* output_time = nullptr;
+
+    OSStatus status = audio_proc(0, in_now, &input_data, &input_time,
+                                 output_data, output_time, proxy_client_data);
+
+    EXPECT_EQ(status, noErr);
+  }
+}
+
 }  // namespace media