From 9fa4a5270c4c649dcd5242f8d11ba6388055e936 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 18 Feb 2020 11:45:55 +0100 Subject: [PATCH 1/3] ply-capslock-icon: Do not draw on free One case where the various widgets are being freed is the pixel-display-s being removed because of a monitor being hot(un)plugged. When the monitor configuration changes ply-device-manager removes all old pixel-displays and then adds the pixel-displays from the new config. Calling ply_pixel_display_draw_area on a pixel-display which is about to be freed is a bad idea, if the monitor was actually unplugged this leads to various sort of errors, including crashes in some cases. ply-capslock-icon is a recently added widget, none of the other (older) widgets redraw themselves as hidden on free because there is no reason to do this. This commit adds a new stop_polling helper and replaces the troublesome hide call (which involves redrawing) with this. This fixes plymouth sometimes crashing when monitors are hot(un)plugged while plymouth is running. Signed-off-by: Hans de Goede --- src/libply-splash-graphics/ply-capslock-icon.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libply-splash-graphics/ply-capslock-icon.c b/src/libply-splash-graphics/ply-capslock-icon.c index 7d19a187..e60457f5 100644 --- a/src/libply-splash-graphics/ply-capslock-icon.c +++ b/src/libply-splash-graphics/ply-capslock-icon.c @@ -52,6 +52,8 @@ struct _ply_capslock_icon bool is_on; }; +static void ply_capslock_stop_polling (ply_capslock_icon_t *capslock_icon); + ply_capslock_icon_t * ply_capslock_icon_new (const char *image_dir) { @@ -74,7 +76,7 @@ ply_capslock_icon_free (ply_capslock_icon_t *capslock_icon) return; if (!capslock_icon->is_hidden) - ply_capslock_icon_hide (capslock_icon); + ply_capslock_stop_polling (capslock_icon); if (capslock_icon->buffer != NULL) ply_pixel_buffer_free (capslock_icon->buffer); @@ -121,6 +123,14 @@ on_timeout (void *user_data, on_timeout, capslock_icon); } +static void +ply_capslock_stop_polling (ply_capslock_icon_t *capslock_icon) +{ + ply_event_loop_stop_watching_for_timeout (capslock_icon->loop, + (ply_event_loop_timeout_handler_t) + on_timeout, capslock_icon); +} + bool ply_capslock_icon_load (ply_capslock_icon_t *capslock_icon) { @@ -183,10 +193,8 @@ ply_capslock_icon_hide (ply_capslock_icon_t *capslock_icon) capslock_icon->is_hidden = true; ply_capslock_icon_draw (capslock_icon); + ply_capslock_stop_polling (capslock_icon); - ply_event_loop_stop_watching_for_timeout (capslock_icon->loop, - (ply_event_loop_timeout_handler_t) - on_timeout, capslock_icon); capslock_icon->loop = NULL; capslock_icon->display = NULL; } From 3fad53add0450777dee48ea9661f569b5dac4c5e Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 18 Feb 2020 11:50:17 +0100 Subject: [PATCH 2/3] ply-keymap-icon: Do not draw on free One case where the various widgets are being freed is the pixel-display-s being removed because of a monitor being hot(un)plugged. When the monitor configuration changes ply-device-manager removes all old pixel-displays and then adds the pixel-displays from the new config. Calling ply_pixel_display_draw_area on a pixel-display which is about to be freed is a bad idea, if the monitor was actually unplugged this leads to various sort of errors, including crashes in some cases. ply-keymap-icon is a recently added widget, none of the other (older) widgets redraw themselves as hidden on free because there is no reason to do this. This commit removes the troublesome hide call (which involves redrawing). This fixes plymouth sometimes crashing when monitors are hot(un)plugged while plymouth is running. Signed-off-by: Hans de Goede --- src/libply-splash-graphics/ply-keymap-icon.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libply-splash-graphics/ply-keymap-icon.c b/src/libply-splash-graphics/ply-keymap-icon.c index f9ec614f..d5375271 100644 --- a/src/libply-splash-graphics/ply-keymap-icon.c +++ b/src/libply-splash-graphics/ply-keymap-icon.c @@ -130,9 +130,6 @@ ply_keymap_icon_free (ply_keymap_icon_t *keymap_icon) if (keymap_icon == NULL) return; - if (!keymap_icon->is_hidden) - ply_keymap_icon_hide (keymap_icon); - ply_pixel_buffer_free (keymap_icon->icon_buffer); ply_pixel_buffer_free (keymap_icon->keymap_buffer); From 1535655ee455c3f1da6ea079aeeb44978e5786ad Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 18 Feb 2020 11:51:03 +0100 Subject: [PATCH 3/3] ply-throbber: Do not redraw when we need to stop throbbing on free One case where the various widgets are being freed is the pixel-display-s being removed because of a monitor being hot(un)plugged. When the monitor configuration changes ply-device-manager removes all old pixel-displays and then adds the pixel-displays from the new config. Calling ply_pixel_display_draw_area on a pixel-display which is about to be freed is a bad idea, if the monitor was actually unplugged this leads to various sort of errors, including crashes in some cases. ply-throbber is the only (older) widget which does a redraw on free, this likely was not noticed until now because typically the throbber will already have been stopped on free. This commit adds a redraw parameter to ply_throbber_stop_now and sets this to false when calling ply_throbber_stop_now from ply_throbber_free. This fixes plymouth sometimes crashing when monitors are hot(un)plugged while plymouth is running. Signed-off-by: Hans de Goede --- src/libply-splash-graphics/ply-throbber.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/libply-splash-graphics/ply-throbber.c b/src/libply-splash-graphics/ply-throbber.c index a4311fda..bf0855ea 100644 --- a/src/libply-splash-graphics/ply-throbber.c +++ b/src/libply-splash-graphics/ply-throbber.c @@ -78,7 +78,7 @@ struct _ply_throbber uint32_t is_stopped : 1; }; -static void ply_throbber_stop_now (ply_throbber_t *throbber); +static void ply_throbber_stop_now (ply_throbber_t *throbber, bool redraw); ply_throbber_t * ply_throbber_new (const char *image_dir, @@ -126,7 +126,7 @@ ply_throbber_free (ply_throbber_t *throbber) return; if (!throbber->is_stopped) - ply_throbber_stop_now (throbber); + ply_throbber_stop_now (throbber, false); ply_throbber_remove_frames (throbber); ply_array_free (throbber->frames); @@ -324,15 +324,18 @@ ply_throbber_start (ply_throbber_t *throbber, } static void -ply_throbber_stop_now (ply_throbber_t *throbber) +ply_throbber_stop_now (ply_throbber_t *throbber, bool redraw) { throbber->is_stopped = true; - ply_pixel_display_draw_area (throbber->display, - throbber->x, - throbber->y, - throbber->frame_area.width, - throbber->frame_area.height); + if (redraw) { + ply_pixel_display_draw_area (throbber->display, + throbber->x, + throbber->y, + throbber->frame_area.width, + throbber->frame_area.height); + } + if (throbber->loop != NULL) { ply_event_loop_stop_watching_for_timeout (throbber->loop, (ply_event_loop_timeout_handler_t) @@ -356,7 +359,7 @@ ply_throbber_stop (ply_throbber_t *throbber, } if (stop_trigger == NULL) { - ply_throbber_stop_now (throbber); + ply_throbber_stop_now (throbber, true); return; }