Commit 2b515b5e authored by P V's avatar P V
Browse files

dbus: keep a ref to DBusConnection if reconnecting is not handled

Several places in the code don't handle reconnecting DBus connections
yet. In those cases, a ref to the DBusConnection handle needs to be
kept, so that there's no use-after-free if it gets freed by spa_dbus
if the connection is broken.

Adjust spa_dbus so that others keeping additional refs is safe.
parent cb6dbd16
Pipeline #348706 passed with stages
in 2 minutes and 58 seconds
......@@ -68,6 +68,11 @@ struct spa_dbus_connection {
/**
* Get the DBusConnection from a wrapper
*
* Note that the returned handle is closed and unref'd by spa_dbus
* immediately before emitting the asynchronous "disconnected" event.
* The caller must either deal with the invalidation, or keep an extra
* ref on the handle returned.
*
* \param conn the spa_dbus_connection wrapper
* \return a pointer of type DBusConnection
*/
......
......@@ -3853,6 +3853,7 @@ static int impl_clear(struct spa_handle *handle)
free((void*)monitor->enabled_codecs.items);
spa_zero(monitor->enabled_codecs);
dbus_connection_unref(monitor->conn);
spa_dbus_connection_destroy(monitor->dbus_connection);
monitor->dbus_connection = NULL;
monitor->conn = NULL;
......@@ -4023,6 +4024,17 @@ impl_init(const struct spa_handle_factory *factory,
return -EIO;
}
this->conn = spa_dbus_connection_get(this->dbus_connection);
if (this->conn == NULL) {
spa_log_error(this->log, "failed to get dbus connection");
spa_dbus_connection_destroy(this->dbus_connection);
this->dbus_connection = NULL;
return -EIO;
}
/* XXX: We should handle spa_dbus reconnecting, but we don't, so ref
* XXX: the handle so that we can keep it if spa_dbus unrefs it.
*/
dbus_connection_ref(this->conn);
spa_hook_list_init(&this->hooks);
......
......@@ -280,6 +280,8 @@ static void wakeup_main(void *userdata)
spa_loop_utils_enable_idle(impl->utils, this->dispatch_event, true);
}
static void connection_close(struct connection *this);
static DBusHandlerResult filter_message (DBusConnection *connection,
DBusMessage *message, void *user_data)
{
......@@ -288,9 +290,7 @@ static DBusHandlerResult filter_message (DBusConnection *connection,
if (dbus_message_is_signal(message, DBUS_INTERFACE_LOCAL, "Disconnected")) {
spa_log_debug(impl->log, "dbus connection %p disconnected", this);
if (this->conn)
dbus_connection_unref(this->conn);
this->conn = NULL;
connection_close(this);
connection_emit_disconnected(this);
}
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
......@@ -348,6 +348,26 @@ error_filter:
return NULL;
}
static void connection_close(struct connection *this)
{
if (this->conn) {
dbus_connection_remove_filter(this->conn, filter_message, this);
dbus_connection_close(this->conn);
/* Someone may still hold a ref to the handle from get(), so the
* unref below may not be the final one. For that case, reset
* all callbacks we defined to be sure they are not called. */
dbus_connection_set_dispatch_status_function(this->conn, NULL, NULL, NULL);
dbus_connection_set_watch_functions(this->conn, NULL, NULL, NULL, NULL, NULL);
dbus_connection_set_timeout_functions(this->conn, NULL, NULL, NULL, NULL, NULL);
dbus_connection_set_wakeup_main_function(this->conn, NULL, NULL, NULL);
dbus_connection_unref(this->conn);
}
this->conn = NULL;
}
static void connection_free(struct connection *conn)
{
struct impl *impl = conn->impl;
......@@ -355,11 +375,7 @@ static void connection_free(struct connection *conn)
spa_list_remove(&conn->link);
if (conn->conn) {
dbus_connection_remove_filter(conn->conn, filter_message, conn);
dbus_connection_close(conn->conn);
dbus_connection_unref(conn->conn);
}
connection_close(conn);
spa_list_consume(data, &conn->source_list, link)
source_data_free(data);
......
......@@ -192,6 +192,9 @@ int main(int argc, char *argv[])
goto exit;
}
/* XXX: we don't handle dbus reconnection yet, so ref the handle instead */
dbus_connection_ref(impl.conn);
impl.device = rd_device_new(impl.conn,
opt_name,
opt_appname,
......@@ -207,6 +210,10 @@ int main(int argc, char *argv[])
rd_device_release(impl.device);
exit:
if (impl.conn)
dbus_connection_unref(impl.conn);
if (impl.dbus)
spa_dbus_connection_destroy(impl.dbus_connection);
if (impl.context)
pw_context_destroy(impl.context);
if (impl.mainloop)
......
......@@ -111,6 +111,8 @@ static void module_destroy(void *data)
spa_hook_remove(&impl->context_listener);
spa_hook_remove(&impl->module_listener);
if (impl->bus)
dbus_connection_unref(impl->bus);
spa_dbus_connection_destroy(impl->conn);
pw_properties_free(impl->properties);
......@@ -240,6 +242,9 @@ static int init_dbus_connection(struct impl *impl)
if (impl->bus == NULL)
return -EIO;
/* XXX: we don't handle dbus reconnection yet, so ref the handle instead */
dbus_connection_ref(impl->bus);
dbus_error_init(&error);
dbus_bus_add_match(impl->bus,
......
......@@ -55,8 +55,10 @@ void *dbus_request_name(struct pw_context *context, const char *name)
return NULL;
bus = spa_dbus_connection_get(conn);
if (bus == NULL)
if (bus == NULL) {
spa_dbus_connection_destroy(conn);
return NULL;
}
dbus_error_init(&error);
......@@ -72,6 +74,8 @@ void *dbus_request_name(struct pw_context *context, const char *name)
dbus_error_free(&error);
spa_dbus_connection_destroy(conn);
errno = EEXIST;
return NULL;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment