Use `spa_loop_invoke()` to invoke a callback on the data loop
to remove an embedded `spa_source` from the data loop.
Embedded `spa_source` objects cannot be safely removed
while the loop is polling without risking potential
use-after-frees.
The core of the issue is the following: what happens if an
active source is destroyed before it could be dispatched?
For loop-managed sources (`struct source_impl`) this was addressed
by storing all destroyed sources in a list, and only freeing them
after dispatching has been finished. (0eb73f0f06)
This approach works for both strictly single-threaded
and `pw_thread_loop` loops assuming the loop is not
reentered.
However, if the loop is reentered, there can still be issues.
Assume that in one iteration sources A and B are active,
and returned from the system call, and source B is destroyed
before the loop starts dispatching. Consider what happens when
"A" is dispatched first, and it reenters the loop with timeout 0.
Imagine there are no new events, so `loop_iterate()` will immediately
return, but it will first destroy everything in the destroy list
(this is done at the end of `loop_iterate()`).
And herein lies the problem. In the previous iteration,
there exists a `spa_poll_event` object which points to source "B",
but that has just been destroyed at the end of the recursive
iteration. This will trigger a use-after-free once the previous
iteration inspects it.
Fix that by processing the destroy list right after first
processing the returned `spa_poll_event` objects, and
"detach" the source from the loop and its iterations
in `process_destroy()` before the source is destroyed.
See #2114#2147
Add a test which triggers two event sources in the loop's "before"
control hook, and destroys the second source in the loop's "after"
control hook, and then reenters the loop in the event handler of
the first source. At the moment, this test triggers a use-after-free.
==2973914==ERROR: AddressSanitizer: heap-use-after-free on address 0x608000000440 [...]
READ of size 4 at 0x608000000440 thread T0
#0 0x7fa97f60c6b7 in loop_iterate ../spa/plugins/support/loop.c:376
#1 0x7fa98472c1eb in pw_main_loop_run ../src/pipewire/main-loop.c:148
#2 0x559995af7a76 in destroy_managed_source_before_dispatch_recurse ../test/test-loop.c:355
#3 0x559995b02678 in start_test_nofork ../test/pwtest.c:882
#4 0x559995b06191 in run_test ../test/pwtest.c:1087
#5 0x559995b0948a in run_tests ../test/pwtest.c:1283
#6 0x559995b0aea4 in main ../test/pwtest.c:1482
#7 0x7fa98360130f in __libc_start_call_main (/usr/lib/libc.so.6+0x2d30f)
#8 0x7fa9836013c0 in __libc_start_main@GLIBC_2.2.5 (/usr/lib/libc.so.6+0x2d3c0)
#9 0x559995aed754 in _start (/home/pb/temp/src/pipewire/build/test/test-loop+0x26754)
0x608000000440 is located 32 bytes inside of 96-byte region [0x608000000420,0x608000000480)
freed by thread T0 here:
#0 0x7fa984ffda79 in __interceptor_free /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
#1 0x7fa97f60b03a in process_destroy ../spa/plugins/support/loop.c:344
#2 0x7fa97f60cbf8 in loop_iterate ../spa/plugins/support/loop.c:387
#3 0x559995af5b62 in dmsbd_recurse_on_event ../test/test-loop.c:298
#4 0x7fa97f60d826 in source_io_func ../spa/plugins/support/loop.c:396
#5 0x7fa97f60c7e7 in loop_iterate ../spa/plugins/support/loop.c:377
#6 0x7fa98472c1eb in pw_main_loop_run ../src/pipewire/main-loop.c:148
#7 0x559995af7a76 in destroy_managed_source_before_dispatch_recurse ../test/test-loop.c:355
#8 0x559995b02678 in start_test_nofork ../test/pwtest.c:882
#9 0x559995b06191 in run_test ../test/pwtest.c:1087
#10 0x559995b0948a in run_tests ../test/pwtest.c:1283
#11 0x559995b0aea4 in main ../test/pwtest.c:1482
#12 0x7fa98360130f in __libc_start_call_main (/usr/lib/libc.so.6+0x2d30f)
previously allocated by thread T0 here:
#0 0x7fa984ffdfb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7fa97f60d883 in loop_add_io ../spa/plugins/support/loop.c:408
#2 0x559995af75de in destroy_managed_source_before_dispatch_recurse ../test/test-loop.c:349
#3 0x559995b02678 in start_test_nofork ../test/pwtest.c:882
#4 0x559995b06191 in run_test ../test/pwtest.c:1087
#5 0x559995b0948a in run_tests ../test/pwtest.c:1283
#6 0x559995b0aea4 in main ../test/pwtest.c:1482
#7 0x7fa98360130f in __libc_start_call_main (/usr/lib/libc.so.6+0x2d30f)
SUMMARY: AddressSanitizer: heap-use-after-free ../spa/plugins/support/loop.c:376 in loop_iterate
Rewrite the test that destroys an active managed source
right after polling is done. There is no need to use a
thread loop because the same thing can be simulated using
the before/after loop control hooks in a more controlled
manner.
It may be a little confusing that both the loop object
and the `source_impl` objects are referred to with variables
named `impl`. For this reason, rename all source_impl objects
named `impl` to `s`.
It is expected that `nfds` is non-negative in the vast majority
of cases, so hopefully the runtime performance will not be
significantly affected by removing the check. This way
it is guaranteed that the destroy list is processed.
Use target-object=<serial/name> instead of path=<id> for specifying
sink/src targets. Deprecate path= argument.
Change device provider to preferably expose serials instead of ids.
Catch SIGTERM/SIGINT in test runner, and clean up any spawned processes.
Fixes printing test output on termination by signal (e.g. meson
timeout), and doesn't leave spawned processes running.
Codec probe connections can trigger bad behavior from oFono if done when
device is busy (e.g. at connect), and they might be done at the same
time as A2DP transport is acquired which cannot work.
Also, oFono will not reply to DBus Acquire, if device does not complete
codec negotiation correctly. This is most likely to happen just after
device connect, when it is busy with other stuff (eg A2DP).
Remove codec probe connections altogether: instead, we guess mSBC if
mSBC is enabled and otherwise CVSD. If the guess turns out to be wrong,
which is unlikely (almost all devices have mSBC), we recreate the
transport with correct codec (from main loop, must not be done in
*_acquire because that can destroy nodes + unload the spa libs while
we're being called from there).
To avoid oFono DBus hangs at startup, add delay before marking the
profile connected, enforcing a time difference to A2DP operations.
