1598413 - Avoid racing issue when play/stop and reinit or destroy stream at the same time

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, defect, P2)

Description

[C.M.Chang[:chunmin]]

The Rust AudioUnit backend has no way to prevent the stream is played or stopped when the stream is being reinitialized or destroyed, while C backend uses a mutex to prevent this problem.

Comment 1

[C.M.Chang[:chunmin]]

Several racing issues:

1. If cubeb_stream_start and cubeb_stream_destroy run in parallel, then the cubeb stream may start playing while it's being destroyed.
   • cubeb_stream_start can be called after stopping the running callback in destroy here
2. If cubeb_stream_stop and cubeb_stream_destroy run in parallel, then the cubeb stream may stop twice (but it‘s probably harmless).
3. If cubeb_stream_start and reinit run in parallel, then cubeb stream may start playing while it's being reinitialized.
   • cubeb_stream_start can be called after checking self.shutdown.load(Ordering::SeqCst) in reinit here
4. If cubeb_stream_stop and reinit run in parallel, then cubeb stream may not stop successfully
   • cubeb_stream_stop can be called after checking self.shutdown.load(Ordering::SeqCst) in reinit here.
5. If cubeb_stream_destroy and reinit run in parallel, then the running callback may not be stopped while it's being destroyed.
   • The code for stopping the running callback in cubeb_stream_destroy here can be executed after reinit here and before starting the existing callbacks here

These could be tested by the manual test added in https://github.com/ChunMinChang/cubeb-coreaudio-rs/pull/24/commits/148e57510bddb589b689852f8843be126dc0b675 , with some thread-sleep added

For 1 and 2, they could be avoided if cubeb_stream_start, cubeb_stream_stop and cubeb_stream_destroy are run in the same task queue in gecko. For 3, 4, 5, we need to fix those in the Rust backend.

Comment 2

[C.M.Chang[:chunmin]]

(In reply to C.M.Chang[:chunmin] from comment #1)
I didn't notice the start or stop may run in parallel with reinit or destroy when defusing the custom mutex (OwnedCriticalSection). The quickest way to fix these may use a Mutex around core_stream_data (revert this).

Comment 3

[C.M.Chang[:chunmin]]

(In reply to C.M.Chang[:chunmin] from comment #2)

(In reply to C.M.Chang[:chunmin] from comment #1)
... fix these may use a Mutex around core_stream_data (revert this).

I changed my mind. This will cause the deadlocks when data-callback thread try to lock core_stream_data in input/output data callback while holding the mutex inside AudioUnit and destory/reinit thread try to call stream-stop that tries to lock the mutex inside AudioUnit while holding the mutex around core_stream_data.

Dispatching stream-start/stop work to the task queue where destroy and reinit are is probably an easier solution. So stream-start/stop, destroy and reinit are all run in the same task queue.

Comment 4

[C.M.Chang[:chunmin]]

Attachment: GitHub Pull Request

Comment 5

[C.M.Chang[:chunmin]]

Attachment: Bug 1598413 - Update cubeb-coreaudio-rs to b6d5b75. r?padenot

Pick commits:
b6d5b75 - Avoid various racing issues (#26)
dba0344 - Add manual tests (#24)

Comment 6

[C.M.Chang[:chunmin]]

D56160 in bug 1601859 imports the #PR 26 (Attachment 9111456 [details])