Serialize all operations talking to the platform, and other fixes #209
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This allows nesting serial queues to ensure all operations are serialized (the top queue) while also ensuring a stream- or context-specific queue cannot run any tasks once its owner has been dropped.
This will be the root queue for all platform audio operations
Per the Apple docs: > As a performance optimization, this function executes blocks on the > current thread whenever possible, with one exception: Blocks submitted > to the main dispatch queue always run on the main thread.
This is to avoid platform bugs resulting in deadlocks and races. It tries to serialize all operations on top of a single serial queue, though probably doesn't cover all. One notable exception is operations in tests that expect a panic, since panics are not forwarded to the calling thread.
This is to avoid deadlocks and races when running tests in CI.
TSAN has complaints otherwise, after running the full suite.
With vpio priming devices may be queried for channel count with a vpio unit around, where they previously wouldn't. The vpio unit existing means output devices may list an extra (input) tap stream. This patch filters out away streams having anything but the most common input terminal types.
padenot
approved these changes
Mar 11, 2024
It allows at most one vpio duplex stream per context, which is hopefully enough for all real-world use cases. Note that Gecko could create at most one duplex stream per document, and there could in theory be multiple documents per context, which is a content process singleton.
Creating the vpio unit takes a long time. This commit lets clients create the shared vpio unit async on a background thread ahead of time, so that when they need it it is already available for reuse.
For input there could be a an output stream if vpio is present on the machine.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This set of patches create a singleton dispatch queue that all other dispatch queues (contexts, streams) sit on top of. In practice this means all calls into CoreAudio are serialized, though not necessarily called from the same thread.
This makes the test suite pass locally on macOS 14, and does seem to help somewhat in CI, although TSAN runs still report an issue with VPIO.
There are also some other fixes and cleanups. The PR is best reviewed patch-by-patch.