Skip to content

Conversation

roderickvd
Copy link
Member

Stream can now be moved between threads safely on macOS.

  • Replaced Rc<RefCell<>> with Arc<Mutex<>> for thread-safe access to stream state
  • Extracted device disconnection handling into a separate DisconnectManager to isolate non-Send AudioObjectPropertyListener
  • Removed unnecessary unsafe impl Send implementations that are now automatically derived

The mutex is only locked during play() and pause() operations, so performance impact is negligible. The audio callback is completely unaffected.

@roderickvd roderickvd changed the title feat: make Stream implement Send on macOS feat(CoreAudio): make Stream implement Send on macOS Sep 27, 2025
@roderickvd roderickvd changed the title feat(CoreAudio): make Stream implement Send on macOS feat(coreaudio): make Stream implement Send on macOS Sep 27, 2025
Copy link
Member

@dvdsk dvdsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect we can not do this without spawning a separate thread to keep AudioObjectPropertyListener in. Maybe its perfectly safe maybe its not. There is not enough documentation from Apple to judge that.

Lets give Apple users an extra thread doing nothing but keep that object on the same thread.... We should name the thread: apple_should_document_their_audio_api \s

@roderickvd
Copy link
Member Author

@dvdsk good catch. Let's be more conservative. I've reworked this to use a dedicated thread.

While I was at it, I also killed the Clone impl on Stream. That's just confusing, and ALSA and WASAPI don't have it either. Should be much cleaner now.

@roderickvd roderickvd requested a review from dvdsk September 28, 2025 18:22
@roderickvd roderickvd changed the title feat(coreaudio): make Stream implement Send on macOS feat(coreaudio): make Stream implement Send on CoreAudio Sep 28, 2025
@roderickvd
Copy link
Member Author

This should be in a good state now - thanks @dvdsk for the necessary reviews. I'll rebase and if you agree, we can merge.

@roderickvd roderickvd force-pushed the feat/coreaudio-send-stream branch 2 times, most recently from 3e765e1 to 25b2135 Compare October 6, 2025 20:42
@edwloef
Copy link
Contributor

edwloef commented Oct 8, 2025

It would be nice if various backends could be tested on their send-ness, to make sure this doesn't accidentally regress. Something on the lines of

const fn assert_send<T: Send>() {}
const _: () = assert_send::<Stream>();

maybe, so that a non-send stream is a compile fail?

@roderickvd
Copy link
Member Author

It would be nice if various backends could be tested on their send-ness, to make sure this doesn't accidentally regress. Something on the lines of

const fn assert_send<T: Send>() {}
const _: () = assert_send::<Stream>();

Adding that in 1d117d3 was pretty revealing: WASAPI and WASM streams aren't Send, either. I'm gonna see if that's easy enough to fix to include with this PR.

@edwloef
Copy link
Contributor

edwloef commented Oct 10, 2025

It would be nice if various backends could be tested on their send-ness, to make sure this doesn't accidentally regress. Something on the lines of

const fn assert_send<T: Send>() {}
const _: () = assert_send::<Stream>();

Adding that in 1d117d3 was pretty revealing: WASAPI and WASM streams aren't Send, either. I'm gonna see if that's easy enough to fix to include with this PR.

On WASM this is probably fine since there's no real concept of threading as far as I'm aware, but the WASAPI stream being Send would be nice.

@roderickvd
Copy link
Member Author

On WASM this is probably fine since there's no real concept of threading as far as I'm aware, but the WASAPI stream being Send would be nice.

You're right. Turned out to be easy for both, so added it here.

@roderickvd roderickvd requested a review from dvdsk October 10, 2025 21:17
@roderickvd roderickvd changed the title feat(coreaudio): make Stream implement Send on CoreAudio feat: make Stream implement Send on all hosts Oct 10, 2025
…ustness

- Use try_lock to avoid panics and recover from poisoned locks when invoking
  error callbacks
- Skip error callback if lock is busy
- Enhance disconnect listener thread to report creation errors and ensure
  proper synchronization
- Update stream lock error message for clarity
Consolidate repeated error callback logic into invoke_error_callback for
better readability and maintainability. Update usages in device.rs and
disconnect manager. Add documentation for the helper.
Removes redundant thread transfer tests for CoreAudio streams.
@roderickvd roderickvd force-pushed the feat/coreaudio-send-stream branch from 4a2231c to 4d23d9e Compare October 13, 2025 19:26
@roderickvd
Copy link
Member Author

Merging this now because I want to start preparing the next release. Please create an issue if something's up with it.

@roderickvd roderickvd merged commit 06a0745 into master Oct 13, 2025
17 checks passed
@roderickvd roderickvd deleted the feat/coreaudio-send-stream branch October 13, 2025 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants