From 425e80adc293a81c5a477312d9f5fc9bfc865389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Sun, 20 Nov 2022 20:17:58 +0100 Subject: [PATCH] display: Freeze stack when closing X11 display If two X11 windows were the last two, we'd remove them from the stack while unmanaging them. That'd hit an assert in meta_stack_tracker_restack_managed(), resulting in the following crash when Xwayland exited unexpectedly with two or more X11 windows being the only windows on the stack: #1 g_assertion_message() at ../glib/gtestutils.c:3256 #2 g_assertion_message_expr() at ../glib/gtestutils.c:3282 #3 meta_stack_tracker_restack_managed() at ../src/core/stack-tracker.c:1210 #4 on_stack_changed() at ../src/core/stack.c:142 #5 _g_closure_invoke_va() at ../gobject/gclosure.c:895 #6 g_signal_emit_valist() at ../gobject/gsignal.c:3456 #7 g_signal_emit() at ../gobject/gsignal.c:3606 #8 meta_stack_changed() at ../src/core/stack.c:265 #9 meta_stack_remove() at ../src/core/stack.c:324 #10 meta_window_unmanage() at ../src/core/window.c:1542 #11 meta_x11_display_unmanage_windows() at ../src/x11/meta-x11-display.c:111 #12 meta_x11_display_dispose() at ../src/x11/meta-x11-display.c:141 #13 g_object_run_dispose() at ../gobject/gobject.c:1448 #14 meta_display_shutdown_x11() at ../src/core/display.c:831 The added test specifically checks that this scenario is handled gracefully. Related: https://bugzilla.redhat.com/show_bug.cgi?id=2143637 Part-of: --- src/core/display.c | 2 ++ src/tests/xwayland-tests.c | 60 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/core/display.c b/src/core/display.c index 906863911..ba78fe04a 100644 --- a/src/core/display.c +++ b/src/core/display.c @@ -826,9 +826,11 @@ meta_display_shutdown_x11 (MetaDisplay *display) if (!display->x11_display) return; + meta_stack_freeze (display->stack); g_signal_emit (display, display_signals[X11_DISPLAY_CLOSING], 0); g_object_run_dispose (G_OBJECT (display->x11_display)); g_clear_object (&display->x11_display); + meta_stack_thaw (display->stack); } MetaDisplay * diff --git a/src/tests/xwayland-tests.c b/src/tests/xwayland-tests.c index 5ead68181..ad115f4aa 100644 --- a/src/tests/xwayland-tests.c +++ b/src/tests/xwayland-tests.c @@ -156,11 +156,71 @@ meta_test_xwayland_restart_selection (void) meta_test_client_destroy (test_client); } +static void +meta_test_xwayland_crash_only_x11 (void) +{ + MetaWaylandCompositor *wayland_compositor = + meta_context_get_wayland_compositor (test_context); + MetaXWaylandManager *xwayland_manager = + meta_wayland_compositor_get_xwayland_manager (wayland_compositor); + MetaDisplay *display = meta_context_get_display (test_context); + MetaTestClient *test_client1; + MetaTestClient *test_client2; + g_autoptr (GError) error = NULL; + + g_assert_null (meta_display_list_all_windows (display)); + + test_client1 = meta_test_client_new (test_context, + "client1", META_WINDOW_CLIENT_TYPE_X11, + &error); + if (!test_client1) + g_error ("Failed to launch test client: %s", error->message); + + test_client2 = meta_test_client_new (test_context, + "client1", META_WINDOW_CLIENT_TYPE_X11, + &error); + if (!test_client2) + g_error ("Failed to launch test client: %s", error->message); + + ensure_xwayland (test_context); + + test_client_do_check (test_client2, "create", "test-window", NULL); + test_client_do_check (test_client1, "create", "test-window", NULL); + test_client_do_check (test_client2, "show", "test-window", NULL); + test_client_do_check (test_client1, "show", "test-window", NULL); + test_client_wait_check (test_client2); + test_client_wait_check (test_client1); + + while (!meta_find_window_from_title (test_context, "test/client1/test-window") || + !meta_find_window_from_title (test_context, "test/client1/test-window")) + g_main_context_iteration (NULL, TRUE); + + g_test_expect_message ("libmutter", G_LOG_LEVEL_WARNING, + "*Connection to xwayland lost*"); + g_test_expect_message ("libmutter", G_LOG_LEVEL_WARNING, + "X Wayland crashed*; attempting to recover"); + + if (!meta_xwayland_signal (xwayland_manager, SIGKILL, &error)) + g_error ("Failed to signal SIGSEGV to Xwayland"); + + while (meta_display_get_x11_display (display)) + g_main_context_iteration (NULL, TRUE); + + g_test_assert_expected_messages (); + + g_assert_null (meta_display_list_all_windows (display)); + + meta_test_client_destroy (test_client1); + meta_test_client_destroy (test_client2); +} + static void init_tests (void) { g_test_add_func ("/backends/xwayland/restart/selection", meta_test_xwayland_restart_selection); + g_test_add_func ("/backends/xwayland/crash/only-x11", + meta_test_xwayland_crash_only_x11); } int