Backport "Fix race hotplug race conditions in DRM lease manager"

Resolves: RHEL-84842
This commit is contained in:
Michel Dänzer 2025-05-21 15:53:26 +02:00 committed by Michel Dänzer
parent d797c19cf7
commit bacb152898
6 changed files with 520 additions and 0 deletions

View File

@ -0,0 +1,47 @@
From 21be0a065c983a1c515414e233813b10944db419 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20=C3=85dahl?= <jadahl@gmail.com>
Date: Mon, 31 Mar 2025 13:49:08 +0200
Subject: [PATCH 1/5] drm-lease: Avoid copying list of connectors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The copied list will be freed, and the copy will be returned, meaning we
can just return the original list directly.
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4378>
(cherry picked from commit e1271f5d546a890c78d8a29e84680adcb73750bc)
---
src/backends/native/meta-drm-lease.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/src/backends/native/meta-drm-lease.c b/src/backends/native/meta-drm-lease.c
index 7cddb6c8ed..a135558a0b 100644
--- a/src/backends/native/meta-drm-lease.c
+++ b/src/backends/native/meta-drm-lease.c
@@ -668,13 +668,6 @@ update_connectors (MetaDrmLeaseManager *lease_manager,
}
}
- for (l = lease_manager->connectors; l; l = l->next)
- {
- kms_connector = l->data;
-
- removed_connectors = g_list_append (removed_connectors, kms_connector);
- }
-
g_hash_table_iter_init (&iter, lease_manager->leased_connectors);
while (g_hash_table_iter_next (&iter, (gpointer *)&kms_connector, NULL))
{
@@ -684,7 +677,7 @@ update_connectors (MetaDrmLeaseManager *lease_manager,
leases_to_revoke = g_list_append (leases_to_revoke, lease);
}
- g_list_free (g_steal_pointer (&lease_manager->connectors));
+ removed_connectors = g_steal_pointer (&lease_manager->connectors);
lease_manager->connectors = new_connectors;
g_clear_pointer (&lease_manager->leased_connectors, g_hash_table_unref);
--
2.49.0

View File

@ -0,0 +1,48 @@
From 6bf9926f1c80ad37c38e0412d11014ca95845ddc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20=C3=85dahl?= <jadahl@gmail.com>
Date: Mon, 31 Mar 2025 13:50:17 +0200
Subject: [PATCH 2/5] drm-lease: Connect MetaUdev::hotplug handler after other
handles
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When we receive a hotplug signal emission, we update the state of leased
and leasable resources. Some of this state depends on the current state
of any potential associated monitor (MetaOutput & MetaMonitor). In order
to have an up to date association of MetaOutput's and MetaMonitor's, we
must only update our own state after MetaMonitorManager has had a chance
to. To achieve this, make sure the MetaUdev::hotplug signal handler is
dispatched after MetaMonitorManager's handler by using
g_signal_connect_after().
Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/3943
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4378>
(cherry picked from commit 3ae8a307ffe2580eff7b5dbaa2fab9ca697e51aa)
---
src/backends/native/meta-drm-lease.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backends/native/meta-drm-lease.c b/src/backends/native/meta-drm-lease.c
index 9b9c6449a0..c6bd2d7380 100644
--- a/src/backends/native/meta-drm-lease.c
+++ b/src/backends/native/meta-drm-lease.c
@@ -871,10 +871,12 @@ meta_drm_lease_manager_constructed (GObject *object)
MetaMonitorManager *monitor_manager =
meta_backend_get_monitor_manager (backend);
+ /* Connect to MetaKms::resources-changed using G_CONNECT_AFTER to make sure
+ * MetaMonitorManager state is up to date. */
lease_manager->resources_changed_handler_id =
- g_signal_connect (kms, "resources-changed",
- G_CALLBACK (on_resources_changed),
- lease_manager);
+ g_signal_connect_after (kms, "resources-changed",
+ G_CALLBACK (on_resources_changed),
+ lease_manager);
lease_manager->lease_changed_handler_id =
g_signal_connect (kms, "lease-changed",
G_CALLBACK (on_lease_changed),
--
2.49.0

View File

@ -0,0 +1,166 @@
From 0c5e21e30ae375fda7a146833c7f412a5c2bd22b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20=C3=85dahl?= <jadahl@gmail.com>
Date: Mon, 31 Mar 2025 13:56:13 +0200
Subject: [PATCH 3/5] tests: Add drm lease test for non non-desktop hotplugs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This tests that https://gitlab.gnome.org/GNOME/mutter/-/issues/3943
doesn't reproduce by triggering hotplugs in a certain way.
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4378>
(cherry picked from commit 2ae93f69eb7228b28519c823cb38b3640a1225e9)
---
src/backends/native/meta-backend-native.h | 2 +
src/tests/drm-lease-monitor-test.c | 104 ++++++++++++++++++++++
src/tests/meson.build | 7 ++
3 files changed, 113 insertions(+)
create mode 100644 src/tests/drm-lease-monitor-test.c
diff --git a/src/backends/native/meta-backend-native.h b/src/backends/native/meta-backend-native.h
index 34115d1ba4..6d63a24966 100644
--- a/src/backends/native/meta-backend-native.h
+++ b/src/backends/native/meta-backend-native.h
@@ -39,8 +39,10 @@ gboolean meta_backend_native_activate_vt (MetaBackendNative *backend_native,
int vt,
GError **error);
+META_EXPORT_TEST
void meta_backend_native_pause (MetaBackendNative *backend_native);
+META_EXPORT_TEST
void meta_backend_native_resume (MetaBackendNative *backend_native);
MetaLauncher * meta_backend_native_get_launcher (MetaBackendNative *native);
diff --git a/src/tests/drm-lease-monitor-test.c b/src/tests/drm-lease-monitor-test.c
new file mode 100644
index 0000000000..d35f5ba83a
--- /dev/null
+++ b/src/tests/drm-lease-monitor-test.c
@@ -0,0 +1,104 @@
+/*
+ * Copyright (C) 2025 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "config.h"
+
+#include <glib.h>
+
+#include "backends/meta-monitor-config-manager.h"
+#include "backends/native/meta-backend-native.h"
+#include "backends/native/meta-kms.h"
+#include "backends/native/meta-kms-device.h"
+#include "meta-test/meta-context-test.h"
+#include "tests/drm-mock/drm-mock.h"
+#include "tests/meta-test-utils.h"
+
+static MetaContext *test_context;
+
+static void
+fake_udev_hotplug (void)
+{
+ MetaBackend *backend = meta_context_get_backend (test_context);
+ MetaBackendNative *backend_native = META_BACKEND_NATIVE (backend);
+ MetaUdev *udev = meta_backend_native_get_udev (backend_native);
+ g_autolist (GUdevDevice) devices = NULL;
+ g_autoptr (GError) error = NULL;
+ GList *l;
+
+ devices = meta_udev_list_drm_devices (udev,
+ META_UDEV_DEVICE_TYPE_CARD,
+ &error);
+ g_assert_no_error (error);
+ g_assert_nonnull (devices);
+
+ for (l = devices; l; l = l->next)
+ {
+ GUdevDevice *device = l->data;
+
+ g_signal_emit_by_name (udev, "hotplug", device);
+ }
+}
+
+static void
+disconnect_connector_filter (gpointer resource,
+ gpointer user_data)
+{
+ drmModeConnector *drm_connector = resource;
+
+ drm_connector->connection = DRM_MODE_DISCONNECTED;
+}
+
+static void
+test_drm_lease_lease_suspend_resume (void)
+{
+ MetaBackend *backend = meta_context_get_backend (test_context);
+ MetaBackendNative *backend_native = META_BACKEND_NATIVE (backend);
+
+ meta_backend_native_pause (backend_native);
+ drm_mock_set_resource_filter (DRM_MOCK_CALL_FILTER_GET_CONNECTOR,
+ disconnect_connector_filter, NULL);
+ fake_udev_hotplug ();
+ meta_backend_native_resume (backend_native);
+
+ drm_mock_unset_resource_filter (DRM_MOCK_CALL_FILTER_GET_CONNECTOR);
+ fake_udev_hotplug ();
+}
+
+static void
+init_tests (void)
+{
+ g_test_add_func ("/wayland/drm-lease/suspend-resume",
+ test_drm_lease_lease_suspend_resume);
+}
+
+int
+main (int argc,
+ char *argv[])
+{
+ g_autoptr (MetaContext) context = NULL;
+
+ context = meta_create_test_context (META_CONTEXT_TEST_TYPE_VKMS,
+ META_CONTEXT_TEST_FLAG_NO_X11);
+ g_assert_true (meta_context_configure (context, &argc, &argv, NULL));
+
+ test_context = context;
+
+ init_tests ();
+
+ return meta_context_test_run_tests (META_CONTEXT_TEST (context),
+ META_TEST_RUN_FLAG_CAN_SKIP);
+}
diff --git a/src/tests/meson.build b/src/tests/meson.build
index 41218e130e..139baad7a1 100644
--- a/src/tests/meson.build
+++ b/src/tests/meson.build
@@ -634,6 +634,13 @@ kms_test_cases = [
wayland_test_utils,
],
},
+ {
+ 'name': 'drm-lease-monitor',
+ 'suite': 'backends/native',
+ 'sources': [
+ 'drm-lease-monitor-test.c',
+ ],
+ },
]
privileged_test_cases += kms_test_cases
--
2.49.0

View File

@ -0,0 +1,81 @@
From 96d4ce647bd73095a5b0789d224271a912582f21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20=C3=85dahl?= <jadahl@gmail.com>
Date: Mon, 31 Mar 2025 14:55:45 +0200
Subject: [PATCH 4/5] output: Add pause/resume signals
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
These gets signalled when the logind session becomes inactive/active,
e.g. VT switches.
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4378>
(cherry picked from commit 288168f7f73ee84208500fe5e6bda6e9cdbc3a39)
---
src/backends/native/meta-backend-native.c | 29 +++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/src/backends/native/meta-backend-native.c b/src/backends/native/meta-backend-native.c
index 85bfeb7810..b81b53d29e 100644
--- a/src/backends/native/meta-backend-native.c
+++ b/src/backends/native/meta-backend-native.c
@@ -79,6 +79,16 @@ enum
static GParamSpec *obj_props[N_PROPS];
+enum
+{
+ PAUSE,
+ RESUME,
+
+ N_SIGNALS
+};
+
+static guint signals[N_SIGNALS];
+
typedef struct _MetaBackendNativePrivate
{
MetaBackend parent;
@@ -869,6 +879,21 @@ meta_backend_native_class_init (MetaBackendNativeClass *klass)
G_PARAM_CONSTRUCT_ONLY |
G_PARAM_STATIC_STRINGS);
g_object_class_install_properties (object_class, N_PROPS, obj_props);
+
+ signals[PAUSE] =
+ g_signal_new ("pause",
+ G_TYPE_FROM_CLASS (klass),
+ G_SIGNAL_RUN_LAST,
+ 0,
+ NULL, NULL, NULL,
+ G_TYPE_NONE, 0);
+ signals[RESUME] =
+ g_signal_new ("resume",
+ G_TYPE_FROM_CLASS (klass),
+ G_SIGNAL_RUN_LAST,
+ 0,
+ NULL, NULL, NULL,
+ G_TYPE_NONE, 0);
}
static void
@@ -961,6 +986,8 @@ meta_backend_native_pause (MetaBackendNative *backend_native)
COGL_TRACE_BEGIN_SCOPED (MetaBackendNativePause,
"Meta::BackendNative::pause()");
+ g_signal_emit (backend_native, signals[PAUSE], 0);
+
meta_seat_native_release_devices (seat);
meta_renderer_pause (renderer);
meta_udev_pause (priv->udev);
@@ -1004,6 +1031,8 @@ void meta_backend_native_resume (MetaBackendNative *native)
meta_input_settings_maybe_restore_numlock_state (input_settings);
clutter_seat_ensure_a11y_state (CLUTTER_SEAT (seat));
+
+ g_signal_emit (native, signals[RESUME], 0);
}
static MetaRenderDevice *
--
2.49.0

View File

@ -0,0 +1,171 @@
From d4f941a053b07b988b79b9f75b2300cadaaf9940 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20=C3=85dahl?= <jadahl@gmail.com>
Date: Mon, 31 Mar 2025 15:02:23 +0200
Subject: [PATCH 5/5] drm-lease: Treat connectors as unleasable when inactive
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When inactive (logind session no longer active, e.g. VT switched to
another user, no longer DRM master) mutter stays dormant and doesn't
try to reconfigure outputs on hotplug events, among other things. That
means we also shouldn't keep leases active, or connectors leasible, when
not active, since dependent state are not kept up to date.
Address this by, treat all connectors as not leasable when inactive,
which effectively means leases are revoked.
This fixes a crash when monitors are hot plugged in a certain way when
the session is inactive, which an added test case tests.
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4378>
(cherry picked from commit eec0f21e40e4319b24f16eb4f97c783b4f559a3e)
---
src/backends/native/meta-drm-lease.c | 36 ++++++++++++++++++++++++++++
src/tests/drm-lease-monitor-test.c | 17 +++++++++++++
2 files changed, 53 insertions(+)
diff --git a/src/backends/native/meta-drm-lease.c b/src/backends/native/meta-drm-lease.c
index c6bd2d7380..6f0c64dae5 100644
--- a/src/backends/native/meta-drm-lease.c
+++ b/src/backends/native/meta-drm-lease.c
@@ -15,6 +15,8 @@
* along with this program; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "backends/native/meta-backend-native-types.h"
+#include "backends/native/meta-backend-native.h"
#include "config.h"
#include "backends/native/meta-drm-lease.h"
@@ -71,6 +73,8 @@ struct _MetaDrmLeaseManager
gulong resources_changed_handler_id;
gulong lease_changed_handler_id;
gulong monitors_changed_handler_id;
+ gulong backend_pause_handler_id;
+ gulong backend_resume_handler_id;
/* MetaKmsDevice *kms_device */
GList *devices;
@@ -84,6 +88,8 @@ struct _MetaDrmLeaseManager
* Value: MetaDrmLease *lease
*/
GHashTable *leased_connectors;
+
+ gboolean is_paused;
};
G_DEFINE_TYPE (MetaDrmLeaseManager, meta_drm_lease_manager, G_TYPE_OBJECT)
@@ -665,6 +671,9 @@ update_connectors (MetaDrmLeaseManager *lease_manager,
new_leased_connectors =
g_hash_table_new_similar (lease_manager->leased_connectors);
+ if (lease_manager->is_paused)
+ goto scanned_resources;
+
for (l = meta_kms_get_devices (kms); l; l = l->next)
{
MetaKmsDevice *kms_device = l->data;
@@ -697,6 +706,8 @@ update_connectors (MetaDrmLeaseManager *lease_manager,
}
}
+scanned_resources:
+
g_hash_table_iter_init (&iter, lease_manager->leased_connectors);
while (g_hash_table_iter_next (&iter, (gpointer *)&kms_connector, NULL))
{
@@ -862,6 +873,21 @@ on_lease_changed (MetaKms *kms,
update_leases (lease_manager);
}
+static void
+on_pause (MetaBackendNative *backend_native,
+ MetaDrmLeaseManager *lease_manager)
+{
+ lease_manager->is_paused = TRUE;
+ update_resources (lease_manager);
+}
+
+static void
+on_resume (MetaBackendNative *backend_native,
+ MetaDrmLeaseManager *lease_manager)
+{
+ lease_manager->is_paused = FALSE;
+}
+
static void
meta_drm_lease_manager_constructed (GObject *object)
{
@@ -870,6 +896,7 @@ meta_drm_lease_manager_constructed (GObject *object)
MetaBackend *backend = meta_kms_get_backend (kms);
MetaMonitorManager *monitor_manager =
meta_backend_get_monitor_manager (backend);
+ MetaBackendNative *backend_native = META_BACKEND_NATIVE (backend);
/* Connect to MetaKms::resources-changed using G_CONNECT_AFTER to make sure
* MetaMonitorManager state is up to date. */
@@ -885,6 +912,10 @@ meta_drm_lease_manager_constructed (GObject *object)
g_signal_connect_swapped (monitor_manager, "monitors-changed-internal",
G_CALLBACK (update_resources),
lease_manager);
+ lease_manager->backend_pause_handler_id =
+ g_signal_connect (backend_native, "pause", G_CALLBACK (on_pause), lease_manager);
+ lease_manager->backend_resume_handler_id =
+ g_signal_connect (backend_native, "resume", G_CALLBACK (on_resume), lease_manager);
lease_manager->leases =
g_hash_table_new_full (NULL, NULL,
@@ -939,11 +970,16 @@ meta_drm_lease_manager_dispose (GObject *object)
MetaBackend *backend = meta_kms_get_backend (kms);
MetaMonitorManager *monitor_manager =
meta_backend_get_monitor_manager (backend);
+ MetaBackendNative *backend_native = META_BACKEND_NATIVE (backend);
g_clear_signal_handler (&lease_manager->resources_changed_handler_id, kms);
g_clear_signal_handler (&lease_manager->lease_changed_handler_id, kms);
g_clear_signal_handler (&lease_manager->monitors_changed_handler_id,
monitor_manager);
+ g_clear_signal_handler (&lease_manager->backend_pause_handler_id,
+ backend_native);
+ g_clear_signal_handler (&lease_manager->backend_resume_handler_id,
+ backend_native);
g_list_free_full (g_steal_pointer (&lease_manager->devices), g_object_unref);
g_list_free_full (g_steal_pointer (&lease_manager->connectors),
diff --git a/src/tests/drm-lease-monitor-test.c b/src/tests/drm-lease-monitor-test.c
index d35f5ba83a..9451036c44 100644
--- a/src/tests/drm-lease-monitor-test.c
+++ b/src/tests/drm-lease-monitor-test.c
@@ -78,11 +78,28 @@ test_drm_lease_lease_suspend_resume (void)
fake_udev_hotplug ();
}
+static void
+test_drm_lease_lease_suspend_no_resume (void)
+{
+ MetaBackend *backend = meta_context_get_backend (test_context);
+ MetaBackendNative *backend_native = META_BACKEND_NATIVE (backend);
+
+ drm_mock_set_resource_filter (DRM_MOCK_CALL_FILTER_GET_CONNECTOR,
+ disconnect_connector_filter, NULL);
+ fake_udev_hotplug ();
+ meta_backend_native_pause (backend_native);
+
+ drm_mock_unset_resource_filter (DRM_MOCK_CALL_FILTER_GET_CONNECTOR);
+ fake_udev_hotplug ();
+}
+
static void
init_tests (void)
{
g_test_add_func ("/wayland/drm-lease/suspend-resume",
test_drm_lease_lease_suspend_resume);
+ g_test_add_func ("/wayland/drm-lease/suspend-no-resume",
+ test_drm_lease_lease_suspend_no_resume);
}
int
--
2.49.0

View File

@ -87,6 +87,13 @@ Patch: 0008-wayland-window-configuration-Add-MetaWindowConfig-su.patch
Patch: 0009-wayland-Emit-the-configure-signal.patch
Patch: 0010-window-x11-Emit-the-configure-signal.patch
# Backport "Fix race hotplug race conditions in DRM lease manager" (RHEL-84842)
Patch: 0001-drm-lease-Avoid-copying-list-of-connectors.patch
Patch: 0002-drm-lease-Connect-MetaUdev-hotplug-handler-after-oth.patch
Patch: 0003-tests-Add-drm-lease-test-for-non-non-desktop-hotplug.patch
Patch: 0004-output-Add-pause-resume-signals.patch
Patch: 0005-drm-lease-Treat-connectors-as-unleasable-when-inacti.patch
BuildRequires: pkgconfig(gobject-introspection-1.0) >= 1.41.0
BuildRequires: pkgconfig(sm)
BuildRequires: pkgconfig(libwacom)