When a link enters the "ERROR" state, it is scheduled for destruction in
`module-link-factory.c:link_state_changed()`, which queues `destroy_link()`
to be executed on the context's work queue.
However, if the link is destroyed by means of `pw_impl_link_destroy()`
directly after that, then `link_destroy()` unregisters the associated
`pw_global`'s event hook, resulting in `global_destroy()` not being called
when `pw_impl_link_destroy()` proceeds to call `pw_global_destroy()` some
time later. This causes the scheduled async work to not be cancelled. When
it runs later, it will trigger a use-after-free since the `link_data` object
is directly tied to the `pw_impl_link` object.
For example, if the link is destroyed when the client disconnects:
==259313==ERROR: AddressSanitizer: heap-use-after-free on address 0x7ce753028af0 at pc 0x7f475354a565 bp 0x7ffd71501930 sp 0x7ffd71501920
READ of size 8 at 0x7ce753028af0 thread T0
#0 0x7f475354a564 in destroy_link ../src/modules/module-link-factory.c:253
#1 0x7f475575a234 in process_work_queue ../src/pipewire/work-queue.c:67
#2 0x7b47504e7f24 in source_event_func ../spa/plugins/support/loop.c:1011
[...]
0x7ce753028af0 is located 1136 bytes inside of 1208-byte region [0x7ce753028680,0x7ce753028b38)
freed by thread T0 here:
#0 0x7f475631f79d in free /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:51
#1 0x7f4755594a44 in pw_impl_link_destroy ../src/pipewire/impl-link.c:1742
#2 0x7f475569dc11 in do_destroy_link ../src/pipewire/impl-port.c:1386
#3 0x7f47556a428b in pw_impl_port_for_each_link ../src/pipewire/impl-port.c:1673
#4 0x7f475569dc3e in pw_impl_port_unlink ../src/pipewire/impl-port.c:1392
#5 0x7f47556a02d8 in pw_impl_port_destroy ../src/pipewire/impl-port.c:1453
#6 0x7f4755634f79 in pw_impl_node_destroy ../src/pipewire/impl-node.c:2447
#7 0x7b474f722ba8 in client_node_resource_destroy ../src/modules/module-client-node/client-node.c:1253
#8 0x7f47556d7c6c in pw_resource_destroy ../src/pipewire/resource.c:325
#9 0x7f475545f07d in destroy_resource ../src/pipewire/impl-client.c:627
#10 0x7f47554550cd in pw_map_for_each ../src/pipewire/map.h:222
#11 0x7f4755460aa4 in pw_impl_client_destroy ../src/pipewire/impl-client.c:681
#12 0x7b474fb0658b in handle_client_error ../src/modules/module-protocol-native.c:471
[...]
Fix this by cancelling the work queue item in `link_destroy()`, which should
always run, regardless of the ordering of events.
Fixes#4691
config.h needs to be consistently included before any standard headers
if we ever want to set feature test macros (like _GNU_SOURCE or whatever)
inside. It can lead to hard-to-debug issues without that.
It can also be problematic just for our own HAVE_* that it may define
if it's not consistently made available before our own headers. Just
always include it first, before everything.
We already did this in many files, just not consistently.
When both node and port are given, check that the port belongs to the
node. If it doesn't, it could be that we found a Port using the
object.id but we should have used the port.id of the node.
Make an option to allow link.passive properties and set it to false by
default.
This effectively ignores the link.passive properties from the session
manager, jack clients and pw-link when set. This is a good idea because
the logic for making passive links is better handled in the core.
Just like the real free() we should just ignore a NULL pointer, makes the
caller code easier for those instances where properties are optional.
Patch generated with concinelle with a few manual fixes.
Make it possible to use object.id, node.name, node.nick,
node.description and object.path to select nodes and object.id,
port.name, port.alias and object.path to select ports when linking.
This makes it possible to use pw-cli creat-link to make nodes
without having to deal with the ids.
Remove the hooks we installed on objects in the destroy/free event
or before calling _destroy. This is not really needed but it is
a nice thing to do because it calls the hook removed callbacks.
Send create_object error messages to the new resource id so that
the client can better know what proxy is failing.
Use resource_remove when create_object fails because the core will
already send a remove_id on failure.
Try to send other errors to the proxy that made the request if
possible.
Sending the errors to the proxy id allows the client to know something
is wrong with the proxy and deal with it better.
Use the port global id in the error message.
Log an error when we send an error to the client so that we don't need
to log and error anymore.
Improve the error messages when we can
Move some warnings and errors to debug
This is more in line with wayland and it allows us to create new
interfaces in modules without having to add anything to the type
enum. It also removes some lookups to map type_id to readable
name in debug.
The proxy API is the one that we would like to expose for applications
and the other API is used internally when implementing modules or
factories.
The current pw_core object is really a context for all objects so
name it that way. It also makes it possible to rename pw_core_proxy
to pw_proxy later.
Bind to the resource in create_object before we register the global.
This ensure a client gets to see the resource global id associated
with the resource before it appears in the registry, which makes it
easier to patch the local proxy to the global object.
