From f816b01a8acf4509e567d7028b4168a8c71058d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20PORTAY?= Date: Wed, 26 Feb 2020 15:57:23 -0500 Subject: [PATCH] main: Add state variable splash_is_becoming_idle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both handlers on_deactivate() and on_quit() sets the handler on_boot_splash_idle() using the helper ply_boot_splash_become_idle(). That helper creates a new trigger and it stores it alongside the handler in its context. The helper asserts if the trigger is set (i.e. a handler is pending already). None of the handlers on_deactivate() and on_quit() check if the handler on_boot_splash_idle() is set already. There is a race condition that leads to the situation in which the assertion is false and causes plymouthd to signal itself with SIGABRT and die. First, a client sends the request deactivate to the daemon that creates a trigger for the idle handler. Then, the trigger is still pending while another client sends the request quit to the daemon that tries to create a second trigger and die because of the assertion. This commit adds the new state variable splash_is_becoming_idle that is checked before calling the helper ply_boot_splash_become_idle() to avoid to call it twice. The state variable is set by the caller after it calls the helper ply_boot_splash_become_idle(). It is cleared by the handler on_boot_splash_idle() after the splash becomes idle. Fixes: Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-boot-server.c:350:print_connection_process_identity : connection is from pid 683 (/usr/bin/plymouth deactivate) with parent pid 1 (/usr/lib/systemd/systemd --switched-root --system --deserialize 27) Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-boot-server.c:459:ply_boot_connection_on_request : got deactivate request Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 main.c:1275:on_deactivate : deactivating Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-event-loop.c:967:ply_event_loop_stop_watching_for_timeout : no matching timeout found for removal Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-device-manager.c:1112:ply_device_manager_pause : ply_device_manager_pause() called, stopping watching for udev events Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-event-loop.c:761:ply_event_loop_stop_watching_fd : stopping watching fd 8 Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-event-loop.c:777:ply_event_loop_stop_watching_fd : removing destination for fd 8 Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-event-loop.c:786:ply_event_loop_stop_watching_fd : no more destinations remaing for fd 8, removing source Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-device-manager.c:1092:ply_device_manager_deactivate_keyboa: deactivating keyboards Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-event-loop.c:761:ply_event_loop_stop_watching_fd : stopping watching fd 10 Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-event-loop.c:777:ply_event_loop_stop_watching_fd : removing destination for fd 10 Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ply-boot-splash.c:687:ply_boot_splash_become_idle : telling splash to become idle Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ./plugin.c:1801:become_idle : deactivation requested Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.186 ./plugin.c:1819:become_idle : already waiting for plugin to stop Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.191 ply-boot-server.c:350:print_connection_process_identity : connection is from pid 685 (/usr/bin/plymouth quit --retain-splash) with parent pid 1 (/usr/lib/systemd/systemd --switched-root --system --deserialize 27) Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.191 ply-boot-server.c:486:ply_boot_connection_on_request : got quit --retain-splash request Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.191 main.c:1326:on_quit : quitting (retain splash: true) Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.191 main.c:1338:on_quit : system initialized so saving boot-duration file Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.191 ply-utils.c:596:ply_create_directory : directory '/var/lib/plymouth/' already exists Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.191 main.c:686:get_cache_file_for_mode : returning cache file '/var/lib/plymouth//boot-duration' Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.191 ply-progress.c:214:ply_progress_save_cache : saving progress cache to /var/lib/plymouth//boot-duration Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.192 main.c:1634:tell_systemd_to_stop_printing_details : telling systemd to stop printing details Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.192 main.c:1352:on_quit : closing log Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.192 ply-device-manager.c:1092:ply_device_manager_deactivate_keyboa: deactivating keyboards Feb 25 22:04:35 steamos plymouthd[312]: 00:00:04.192 main.c:1358:on_quit : unloading splash Feb 25 22:04:35 steamos plymouthd[312]: plymouthd: ply-boot-splash.c:677: ply_boot_splash_become_idle: Assertion `splash->idle_trigger == NULL' failed. Feb 25 22:05:06 steamos systemd[1]: plymouth-start.service: Main process exited, code=dumped, status=6/ABRT Feb 25 22:05:06 steamos systemd[1]: plymouth-start.service: Failed with result 'core-dump'. Signed-off-by: Gaël PORTAY --- src/main.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/main.c b/src/main.c index 01a7ee2..a051f0f 100644 --- a/src/main.c +++ b/src/main.c @@ -108,6 +108,7 @@ typedef struct uint32_t is_inactive : 1; uint32_t is_shown : 1; uint32_t should_force_details : 1; + uint32_t splash_is_becoming_idle : 1; char *override_splash_path; char *system_default_splash_path; @@ -1250,6 +1251,8 @@ on_boot_splash_idle (state_t *state) ply_trace ("deactivating splash"); deactivate_splash (state); } + + state->splash_is_becoming_idle = false; } static void @@ -1279,10 +1282,13 @@ on_deactivate (state_t *state, ply_device_manager_deactivate_keyboards (state->device_manager); if (state->boot_splash != NULL) { - ply_boot_splash_become_idle (state->boot_splash, - (ply_boot_splash_on_idle_handler_t) - on_boot_splash_idle, - state); + if (!state->splash_is_becoming_idle) { + ply_boot_splash_become_idle (state->boot_splash, + (ply_boot_splash_on_idle_handler_t) + on_boot_splash_idle, + state); + state->splash_is_becoming_idle = true; + } } else { ply_trace ("deactivating splash"); deactivate_splash (state); @@ -1362,10 +1368,13 @@ on_quit (state_t *state, dump_details_and_quit_splash (state); quit_program (state); } else if (state->boot_splash != NULL) { - ply_boot_splash_become_idle (state->boot_splash, - (ply_boot_splash_on_idle_handler_t) - on_boot_splash_idle, - state); + if (!state->splash_is_becoming_idle) { + ply_boot_splash_become_idle (state->boot_splash, + (ply_boot_splash_on_idle_handler_t) + on_boot_splash_idle, + state); + state->splash_is_becoming_idle = true; + } } else { quit_program (state); }