The `impl::servers` list contains all servers, and they are all
destroyed in `impl_clear()`. However, by that time, modules were
not freed previously, so if there were any instances of
*module-protocol-native-tcp* loaded, then the unload() method
of those would call `server_free()` on already freed servers,
resulting in use-after-frees. Fix that by unloading modules
before destroying the servers.
==451490==ERROR: AddressSanitizer: heap-use-after-free on address 0x612000006050 ...
READ of size 8 at 0x612000006050 thread T0
#0 0x7f45edb19a0c in server_free ../src/modules/module-protocol-pulse/server.c:1022
#1 0x7f45edb46c7d in module_native_protocol_tcp_unload ../src/modules/module-protocol-pulse/modules/module-native-protocol-tcp.c:66
#2 0x7f45eda893c7 in module_unload ../src/modules/module-protocol-pulse/module.c:128
#3 0x7f45edaf7269 in impl_unload_module ../src/modules/module-protocol-pulse/pulse-server.c:5336
#4 0x7f45edaa1583 in pw_map_for_each ../src/pipewire/map.h:238
#5 0x7f45edaf79c5 in impl_clear ../src/modules/module-protocol-pulse/pulse-server.c:5358
...
0x612000006050 is located 16 bytes inside of 264-byte region [0x612000006040,0x612000006148)
freed by thread T0 here:
#0 0x7f45f30be672 in __interceptor_free /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:52
#1 0x7f45edb1a48a in server_free ../src/modules/module-protocol-pulse/server.c:1040
#2 0x7f45edaf730b in impl_clear ../src/modules/module-protocol-pulse/pulse-server.c:5347
...
Fixes: f181210b29 ("pulse-server: properly unload modules")
Check all pending streams to see if the new link completes their
creation.
It is possible that the link linked 2 pending streams and then we
want to complete them both.
Make sure to never send less than the negotiated fragsize to a client.
Also make sure we don't send too much data in one go. This is more in
line with what pulseaudio does.
Fixes capture from multiple tabs in chrome.
Fixes#2418
Make MOVE_SINK_INPUT/MOVE_SOURCE_OUTPUT change the linked peer
immediately in subsequent GET_SINK_INPUT_INFO/GET_SOURCE_OUTPUT_INFO
commands. Do this by keeping track of the sink/source where the client
moved the stream to, and temporarily replying so in future GET_INFO (but
only in messages for that client).
We discard the temporary override when we either get an update event for
the stream (i.e. SM moved the stream), or a 1sec timer runs out. If the
timer runs out, we emit a sink-input/source-output change event, as in
that case what we claimed in the earlier GET_INFO messages might not be
true, so clients need to update their information.
This gets rid of race conditions where an application moves a stream,
and expects the move to be visible in future GET_INFO replies, which may
fail to happen because it takes some time for the session manager to
re-link the streams.
Fixes pasystray behavior.
This prevents the ringbuffer from advancing the read pointer more than
the size of data actually available. It prevents the "avail" value from
keeping to drop when no audio is being played by the client.
Applying this patch seems to prevent the "randomly playing music after
a couple hours of silence" issue
Fixes#2366
Guess the expected latency with the stream info we have and use that as
the node.latency. This way, the graph can attempt to start with some
sort of latency setting.
After we know the exact format, we can calculate the real latency and we
will update the node latency accordingly.
Move all module methods into the `module_info` struct, and place
all such structs into the "pw_mod_pulse_modules" section of
the executable. This way there is no need for an explicit
module registry, and all information about a module can
be declared in the module's source file in a single place.
Deprecate pw_stream_get_time() in favour of _get_time_n() that contains
the size of the pw_time structure. Make the old one fill in the fields
up to the buffered field. Make the new one use the size to decide how
much info to fill in.
Add a new buffered field in pw_time that contains the buffered data
inside the converter/resampler. This leaves the queued field with
purely the user provided size in the buffers.
Use get_time_n() in places.
We need to create fake channels when parsing the IEC958 format or
else we get an invalid format and IEC958 passthrough doesn't work.
Ignore the IEC958 formats when collecting formats for the device or else
the fake channels mess with the real channels of the device.
See #1442
PulseAudio will convert the samplespec/channelmap to a format_info
and omits the input format/rate/channels when the fix flags are set. It
does not use the input values at all.
Do the same in pipewire-pulse, make a single format description without
the requested fields.
This also needs a session manager fix to make it deal with those missing
fields.
See #876
Wait the reply to be sent and the sample to complete playback before
freeing the sample. Otherwise it might have been possible that the
sample completes before the reply can be sent.
If the sample finished playing before we finished the roundtrip to
get the sink_index, it will be destroyed. When the roundtrip completes,
it will try to use invalid memoryy and crash.
Make sure we destroy all pending replies before destroying the sample
to avoid this problem.
Fixes#2151
Operations sync manager, so use that.
On Pulseaudio, module unload is async procedure. However, this race
condition may be hard to hit in practice, whereas on pipewire-pulse it's
not hard. E.g. Zoom appears to assume that modules are unloaded
synchronously.
Sync client's manager, before returning from module load, also for async
module loads. This is because the module load may have its own core, so
even though it has made changes on server, the client manager might not
see them yet.
Fix this by syncing client->manager before operation return.
For the modules that we load internally, place a pulse.module.id
property on the nodes.
If there is no module.id property on nodes, try to use the
pulse.module.id when introspecting. We should not remap those to
serial numbers.
Fixes#2101
Most feature checks already use #ifdef, and do not care about
the value of the macro. Convert all feature checks to do that,
and simplify the meson build scripts by replacing
if cond
cdata.set('X', 1)
endif
with
cdata.set('X', cond)
When the pulse server context is being freed, only a single reference
should be alive to each sample because the pending samples
are all destroyed beforehand.
Clean everything up when the context is destroyed to avoid
problems stemming from the destruction order of objects
in the context.
Furthermore, free messages after all clients have been freed
because `client_free()` may very well call `message_free()`
which would lead to memory leaks if `impl->free_messages` were
processed first.
A stream's lifetime is tied to that of the client, it cannot outlive
the client object. Streams also do not increase the client's reference
count. It is not possible for a stream to exist after the corresponding
client has been destroyed.
Hence it is not necessary to manage the client's reference count
or lifetime in `do_destroy_stream()`. All works related to a particular
stream are cancelled in `stream_free()`, which is called
before the client is destroyed.
When the `done` flag was first added in
9f9be7d7f2, it was
actually needed because cleanup was implemented
using a per-client eventfd which was signalled when
something related to the particular client needed
to be freed. The function that ran, then, checked
each stream's `done` flag, and freed them as necessary.
However, since c70a5de526,
the stream cleanup is done using a work queue, and as
a consequence, the `done` flag is no longer needed.
When the `disconnecting` flag was first added in
d44fdabea1, a client's
streams were not explicitly destroyed when it was freed,
instead, only the client's pw_core was disconnected,
which then caused all related streams to be destroyed.
Thus there needed to be a way to determine why the stream's
state changed to PW_STREAM_STATE_UNCONNECTED as it may have
been because the client is disconnecting or because the
stream has been "killed" in some other way.
The `COMMAND_{PLAYBACK,RECORD}_STREAM_KILLED` command
only needed to be sent to the client in the second case.
However, at the moment, all streams of a client are explicitly
destroyed before it's pw_core is disconnected. This makes the
`disconnecting` flag unnecessary, thus it can be removed.
Move the reference counting from `pending_sample_free()`
into `on_sample_done()` so that the client's references
are managed in a single place.
The reason why `pending_sample_free()` cannot simply call
`client_unref()` is that `client_free()` may be called from
`manager_disconnect()` regardless of the reference count,
and, in turn, `pending_sample_free()` may be called,
which could then lead to a recursive call to `client_free()`.
