Changeset c77cfd8 in mainline

Timestamp:

2022-10-04T19:55:25Z (19 months ago)

Author:

Jiri Svoboda <jiri@…>

Branches:

master, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

e0e612b

Parents:

3fd38b2

Message:

Proper timer cleanup, unit tests

Destroying a timer that sets itself again (i.e., runs periodically) is
a little tricky. It requires a handshake (similar to destroying a worker
thread). Realizing that this handshake works correctly is a little bit
mind boggling.

Location:

uspace/app/taskbar

Files:

• clock.c (modified) (3 diffs)
• meson.build (modified) (1 diff)
• taskbar.c (modified) (2 diffs)
• test/clock.c (added)
• test/main.c (added)
• test/taskbar.c (added)
• types/clock.h (modified) (1 diff)

uspace/app/taskbar/clock.c

@@ -89 +89 @@
         }
 
+        fibril_mutex_initialize(&clock->lock);
+        fibril_condvar_initialize(&clock->timer_done_cv);
         fibril_timer_set(clock->timer, 1000000, taskbar_clock_timer, clock);
 
@@ -106 +108 @@
 void taskbar_clock_destroy(taskbar_clock_t *clock)
 {
+        /*
+         * Signal to the timer that we are cleaning up. If the timer handler
+         * misses it and sets the timer again, we will clear that active
+         * timer and be done (and if we were even slower and the timer
+         * fired again, it's the same situation as before.
+         */
+        fibril_mutex_lock(&clock->lock);
+        clock->timer_cleanup = true;
+        fibril_mutex_unlock(&clock->lock);
+
+        /* If we catch the timer while it's active, there's nothing to do. */
+        if (fibril_timer_clear(clock->timer) != fts_active) {
+                /* Need to wait for timer handler to finish */
+                fibril_mutex_lock(&clock->lock);
+                while (clock->timer_done == false)
+                        fibril_condvar_wait(&clock->timer_done_cv, &clock->lock);
+                fibril_mutex_unlock(&clock->lock);
+        }
+
         fibril_timer_destroy(clock->timer);
         ui_control_delete(clock->control);
@@ -302 +323 @@
         taskbar_clock_t *clock = (taskbar_clock_t *) arg;
 
+        fibril_mutex_lock(&clock->lock);
         (void) taskbar_clock_paint(clock);
-        fibril_timer_set(clock->timer, 1000000, taskbar_clock_timer, clock);
+
+        if (!clock->timer_cleanup) {
+                fibril_timer_set(clock->timer, 1000000, taskbar_clock_timer,
+                    clock);
+        } else {
+                /* Acknowledge timer cleanup */
+                clock->timer_done = true;
+                fibril_condvar_signal(&clock->timer_done_cv);
+        }
+
+        fibril_mutex_unlock(&clock->lock);
 }
 

uspace/app/taskbar/meson.build

@@ -33 +33 @@
         'taskbar.c',
 )
+
+test_src = files(
+        'clock.c',
+        'taskbar.c',
+        'test/clock.c',
+        'test/main.c',
+        'test/taskbar.c',
+)

uspace/app/taskbar/taskbar.c

@@ -92 +92 @@
         rc = ui_get_rect(taskbar->ui, &scr_rect);
         if (rc != EOK) {
-                printf("Error getting screen dimensions.\n");
-                goto error;
+                if (str_cmp(display_spec, UI_DISPLAY_NULL) != 0) {
+                        printf("Error getting screen dimensions.\n");
+                        goto error;
+                }
+
+                /* For the sake of unit tests */
+                scr_rect.p0.x = 0;
+                scr_rect.p0.y = 0;
+                scr_rect.p1.x = 100;
+                scr_rect.p1.y = 100;
         }
 
@@ -194 +202 @@
 void taskbar_destroy(taskbar_t *taskbar)
 {
+        ui_fixed_remove(taskbar->fixed,  taskbar_clock_ctl(taskbar->clock));
         taskbar_clock_destroy(taskbar->clock);
         ui_window_destroy(taskbar->window);

uspace/app/taskbar/types/clock.h

@@ -55 +55 @@
         gfx_rect_t rect;
 
+        /** Clock lock */
+        fibril_mutex_t lock;
+
         /** Clock update timer */
         fibril_timer_t *timer;
+
+        /** Signal to timer that we are cleaning up */
+        bool timer_cleanup;
+
+        /** Signal to maih thread that timer is done */
+        bool timer_done;
+
+        /** Timer done condition variable */
+        fibril_condvar_t timer_done_cv;
 } taskbar_clock_t;