Fix timer setTimeout issue and some coding style issues (#459)

core/app-framework/base/native/timer_wrapper.c

@@ -8,17 +8,19 @@
 #include "../app-manager/module_wasm_app.h"
 #include "timer_native_api.h"
 
-static bool timer_thread_run = true;
-
-bh_list g_timer_ctx_list;
-korp_cond g_timer_ctx_list_cond;
-korp_mutex g_timer_ctx_list_mutex;
 typedef struct {
     bh_list_link l;
     timer_ctx_t timer_ctx;
 } timer_ctx_node_t;
 
-void wasm_timer_callback(timer_id_t id, unsigned int mod_id)
+static bool timer_thread_run = true;
+
+static bh_list g_timer_ctx_list;
+static korp_cond g_timer_ctx_list_cond;
+static korp_mutex g_timer_ctx_list_mutex;
+
+void
+wasm_timer_callback(timer_id_t id, unsigned int mod_id)
 {
     module_data* module = module_data_list_lookup_id(mod_id);
     if (module == NULL)
@@ -29,13 +31,14 @@ void wasm_timer_callback(timer_id_t id, unsigned int mod_id)
     bh_post_msg(module->queue, TIMER_EVENT_WASM, (char *)(uintptr_t)id, 0);
 }
 
-///
-/// why we create a separate link for module timer contexts
-/// rather than traverse the module list?
-/// It helps to reduce the lock frequency for the module list.
-/// Also when we lock the module list and then call the callback for
-/// timer expire, the callback is request the list lock again for lookup
-/// the module from module id. It is for avoiding that situation.
+/**
+ * why we create a separate link for module timer contexts
+ * rather than traverse the module list?
+ * It helps to reduce the lock frequency for the module list.
+ * Also when we lock the module list and then call the callback for
+ * timer expire, the callback is request the list lock again for lookup
+ * the module from module id. It is for avoiding that situation.
+ */
 
 void * thread_modulers_timer_check(void * arg)
 {
@@ -68,37 +71,41 @@ void * thread_modulers_timer_check(void * arg)
     return NULL;
 }
 
-void wakeup_modules_timer_thread(timer_ctx_t ctx)
+void
+wakeup_modules_timer_thread(timer_ctx_t ctx)
 {
     os_mutex_lock(&g_timer_ctx_list_mutex);
     os_cond_signal(&g_timer_ctx_list_cond);
     os_mutex_unlock(&g_timer_ctx_list_mutex);
 }
 
-void init_wasm_timer()
+void
+init_wasm_timer()
 {
     korp_tid tm_tid;
     bh_list_init(&g_timer_ctx_list);
 
     os_cond_init(&g_timer_ctx_list_cond);
-    /* temp solution for: thread_modulers_timer_check thread would recursive lock the mutex */
+    /* temp solution for: thread_modulers_timer_check thread
+       would recursive lock the mutex */
     os_recursive_mutex_init(&g_timer_ctx_list_mutex);
 
     os_thread_create(&tm_tid, thread_modulers_timer_check,
                      NULL, BH_APPLET_PRESERVED_STACK_SIZE);
 }
 
-void exit_wasm_timer()
+void
+exit_wasm_timer()
 {
     timer_thread_run = false;
 }
 
-timer_ctx_t create_wasm_timer_ctx(unsigned int module_id, int prealloc_num)
+timer_ctx_t
+create_wasm_timer_ctx(unsigned int module_id, int prealloc_num)
 {
     timer_ctx_t ctx = create_timer_ctx(wasm_timer_callback,
                                        wakeup_modules_timer_thread,
-                                       prealloc_num,
-                                       module_id);
+                                       prealloc_num, module_id);
 
     if (ctx == NULL)
         return NULL;
@@ -119,11 +126,14 @@ timer_ctx_t create_wasm_timer_ctx(unsigned int module_id, int prealloc_num)
     return ctx;
 }
 
-void destroy_module_timer_ctx(unsigned int module_id)
+void
+destroy_module_timer_ctx(unsigned int module_id)
 {
+    timer_ctx_node_t* elem;
+
     os_mutex_lock(&g_timer_ctx_list_mutex);
-    timer_ctx_node_t* elem = (timer_ctx_node_t*)
-                             bh_list_first_elem(&g_timer_ctx_list);
+    elem = (timer_ctx_node_t*)
+           bh_list_first_elem(&g_timer_ctx_list);
     while (elem) {
         if (timer_ctx_get_owner(elem->timer_ctx) == module_id) {
             bh_list_remove(&g_timer_ctx_list, elem);
@@ -137,7 +147,8 @@ void destroy_module_timer_ctx(unsigned int module_id)
     os_mutex_unlock(&g_timer_ctx_list_mutex);
 }
 
-timer_ctx_t get_wasm_timer_ctx(wasm_module_inst_t module_inst)
+timer_ctx_t
+get_wasm_timer_ctx(wasm_module_inst_t module_inst)
 {
     module_data * m = app_manager_get_module_data(Module_WASM_App,
                                                   module_inst);
@@ -184,8 +195,6 @@ wasm_timer_restart(wasm_exec_env_t exec_env,
     sys_timer_restart(timer_ctx, timer_id, interval);
 }
 
-extern uint32 get_sys_tick_ms();
-
 uint32
 wasm_get_sys_tick_ms(wasm_exec_env_t exec_env)
 {

core/app-mgr/app-manager/module_utils.c

@@ -198,17 +198,16 @@ void release_module(module_data *m_data)
     APP_MGR_FREE(m_data);
 }
 
-int check_modules_timer_expiry()
+uint32 check_modules_timer_expiry()
 {
     os_mutex_lock(&module_data_list_lock);
     module_data *p = module_data_list;
-    int ms_to_expiry = -1;
+    uint32 ms_to_expiry = (uint32)-1;
 
     while (p) {
-
-        int next = get_expiry_ms(p->timer_ctx);
-        if (next != -1) {
-            if (ms_to_expiry == -1 || ms_to_expiry > next)
+        uint32 next = get_expiry_ms(p->timer_ctx);
+        if (next != (uint32)-1) {
+            if (ms_to_expiry == (uint32)-1 || ms_to_expiry > next)
                 ms_to_expiry = next;
         }
 

core/shared/utils/runtime_timer.c

@@ -5,101 +5,111 @@
 
 #include "runtime_timer.h"
 
-#define PRINT(...)
-//#define PRINT printf
+#if 1
+#define PRINT(...) (void)0
+#else
+#define PRINT printf
+#endif
 
 typedef struct _app_timer {
     struct _app_timer * next;
     uint32 id;
-    unsigned int interval;
+    uint32 interval;
     uint64 expiry;
     bool is_periodic;
 } app_timer_t;
 
-struct _timer_ctx {
-    app_timer_t * g_app_timers;
-    app_timer_t * idle_timers;
-    app_timer_t * free_timers;
-    unsigned int g_max_id;
+typedef struct _timer_ctx {
+    app_timer_t *app_timers;
+    app_timer_t *idle_timers;
+    app_timer_t *free_timers;
+    uint32 max_timer_id;
     int pre_allocated;
-    unsigned int owner;
+    uint32 owner;
 
-    //add mutext and conditions
+    /* mutex and condition */
     korp_cond cond;
     korp_mutex mutex;
 
     timer_callback_f timer_callback;
     check_timer_expiry_f refresh_checker;
-};
+} *timer_ctx_t;
 
-uint64 bh_get_tick_ms()
+uint64
+bh_get_tick_ms()
 {
     return os_time_get_boot_microsecond() / 1000;
 }
 
-uint32 bh_get_elpased_ms(uint32 * last_system_clock)
+uint32
+bh_get_elpased_ms(uint32 *last_system_clock)
 {
     uint32 elpased_ms;
-
-    // attention: the bh_get_tick_ms() return 64 bits integer.
-    // but the bh_get_elpased_ms() is designed to use 32 bits clock count.
+    /* attention: the bh_get_tick_ms() return 64 bits integer, but
+       the bh_get_elpased_ms() is designed to use 32 bits clock count */
     uint32 now = (uint32)bh_get_tick_ms();
 
-    // system clock overrun
+    /* system clock overrun */
     if (now < *last_system_clock) {
-        elpased_ms = now + (0xFFFFFFFF - *last_system_clock) + 1;
-    } else {
+        PRINT("system clock overrun!\n");
+        elpased_ms = now + (UINT32_MAX - *last_system_clock) + 1;
+    }
+    else {
         elpased_ms = now - *last_system_clock;
     }
 
     *last_system_clock = now;
-
     return elpased_ms;
 }
 
-static app_timer_t * remove_timer_from(timer_ctx_t ctx, uint32 timer_id,
-                                       bool active_list)
+static app_timer_t *
+remove_timer_from(timer_ctx_t ctx, uint32 timer_id, bool active_list)
 {
+    app_timer_t **head, *prev, *t;
+
     os_mutex_lock(&ctx->mutex);
-    app_timer_t ** head;
+
     if (active_list)
-        head = &ctx->g_app_timers;
+        head = &ctx->app_timers;
     else
         head = &ctx->idle_timers;
 
-    app_timer_t * t = *head;
-    app_timer_t * prev = NULL;
+    t = *head;
+    prev = NULL;
 
     while (t) {
         if (t->id == timer_id) {
             if (prev == NULL) {
                 *head = t->next;
-                PRINT("removed timer [%d] at head from list %d\n", t->id, active_list);
-            } else {
+                PRINT("removed timer [%d] at head from list %d\n",
+                      t->id, active_list);
+            }
+            else {
                 prev->next = t->next;
-                PRINT("removed timer [%d] after [%d] from list %d\n", t->id, prev->id, active_list);
+                PRINT("removed timer [%d] after [%d] from list %d\n",
+                      t->id, prev->id, active_list);
             }
             os_mutex_unlock(&ctx->mutex);
 
             if (active_list && prev == NULL && ctx->refresh_checker)
                 ctx->refresh_checker(ctx);
-
             return t;
-        } else {
+        }
+        else {
             prev = t;
             t = t->next;
         }
     }
 
     os_mutex_unlock(&ctx->mutex);
-
     return NULL;
 }
 
-static app_timer_t * remove_timer(timer_ctx_t ctx, uint32 timer_id,
-                                  bool * active)
+static app_timer_t *
+remove_timer(timer_ctx_t ctx, uint32 timer_id, bool *active)
 {
-    app_timer_t* t = remove_timer_from(ctx, timer_id, true);
+    app_timer_t *t = remove_timer_from(ctx, timer_id, true);
+
     if (t) {
         if (active)
             *active = true;
@@ -111,61 +121,63 @@ static app_timer_t * remove_timer(timer_ctx_t ctx, uint32 timer_id,
     return remove_timer_from(ctx, timer_id, false);
 }
 
-static void reschedule_timer(timer_ctx_t ctx, app_timer_t * timer)
+static void
+reschedule_timer(timer_ctx_t ctx, app_timer_t *timer)
 {
+    app_timer_t *t;
+    app_timer_t *prev = NULL;
 
     os_mutex_lock(&ctx->mutex);
-    app_timer_t * t = ctx->g_app_timers;
-    app_timer_t * prev = NULL;
 
+    t = ctx->app_timers;
     timer->next = NULL;
     timer->expiry = bh_get_tick_ms() + timer->interval;
 
     while (t) {
         if (timer->expiry < t->expiry) {
             if (prev == NULL) {
-                timer->next = ctx->g_app_timers;
-                ctx->g_app_timers = timer;
+                timer->next = ctx->app_timers;
+                ctx->app_timers = timer;
                 PRINT("rescheduled timer [%d] at head\n", timer->id);
-            } else {
+            }
+            else {
                 timer->next = t;
                 prev->next = timer;
-                PRINT("rescheduled timer [%d] after [%d]\n", timer->id, prev->id);
+                PRINT("rescheduled timer [%d] after [%d]\n",
+                      timer->id, prev->id);
             }
 
-            os_mutex_unlock(&ctx->mutex);
-
-            // ensure the refresh_checker() is called out of the lock
-            if (prev == NULL && ctx->refresh_checker)
-                ctx->refresh_checker(ctx);
-
-            return;
-        } else {
+            goto out;
+        }
+        else {
             prev = t;
             t = t->next;
         }
     }
 
     if (prev) {
-        // insert to the list end
+        /* insert to the list end */
         prev->next = timer;
-        PRINT("rescheduled timer [%d] at end, after [%d]\n", timer->id, prev->id);
-    } else {
-        // insert at the begin
-        bh_assert(ctx->g_app_timers == NULL);
-        ctx->g_app_timers = timer;
+        PRINT("rescheduled timer [%d] at end, after [%d]\n",
+              timer->id, prev->id);
+    }
+    else {
+        /* insert at the begin */
+        bh_assert(ctx->app_timers == NULL);
+        ctx->app_timers = timer;
         PRINT("rescheduled timer [%d] as first\n", timer->id);
     }
 
+out:
     os_mutex_unlock(&ctx->mutex);
 
-    // ensure the refresh_checker() is called out of the lock
+    /* ensure the refresh_checker() is called out of the lock */
     if (prev == NULL && ctx->refresh_checker)
         ctx->refresh_checker(ctx);
-
 }
 
-static void release_timer(timer_ctx_t ctx, app_timer_t * t)
+static void
+release_timer(timer_ctx_t ctx, app_timer_t *t)
 {
     if (ctx->pre_allocated) {
         os_mutex_lock(&ctx->mutex);
@@ -173,15 +185,18 @@ static void release_timer(timer_ctx_t ctx, app_timer_t * t)
         ctx->free_timers = t;
         PRINT("recycle timer :%d\n", t->id);
         os_mutex_unlock(&ctx->mutex);
-    } else {
+    }
+    else {
         PRINT("destroy timer :%d\n", t->id);
         BH_FREE(t);
     }
 }
 
-void release_timer_list(app_timer_t ** p_list)
+void
+release_timer_list(app_timer_t **p_list)
 {
     app_timer_t *t = *p_list;
+
     while (t) {
         app_timer_t *next = t->next;
         PRINT("destroy timer list:%d\n", t->id);
@@ -193,18 +208,19 @@ void release_timer_list(app_timer_t ** p_list)
 }
 
 /*
- *
- *    API exposed
- *
+ * API exposed
  */
 
-timer_ctx_t create_timer_ctx(timer_callback_f timer_handler,
-                             check_timer_expiry_f expiery_checker, int prealloc_num,
-                             unsigned int owner)
+timer_ctx_t
+create_timer_ctx(timer_callback_f timer_handler,
+                 check_timer_expiry_f expiery_checker,
+                 int prealloc_num, unsigned int owner)
 {
-    timer_ctx_t ctx = (timer_ctx_t) BH_MALLOC(sizeof(struct _timer_ctx));
+    timer_ctx_t ctx = (timer_ctx_t)BH_MALLOC(sizeof(struct _timer_ctx));
+
     if (ctx == NULL)
         return NULL;
+
     memset(ctx, 0, sizeof(struct _timer_ctx));
 
     ctx->timer_callback = timer_handler;
@@ -213,7 +229,8 @@ timer_ctx_t create_timer_ctx(timer_callback_f timer_handler,
     ctx->owner = owner;
 
     while (prealloc_num > 0) {
-        app_timer_t *timer = (app_timer_t*) BH_MALLOC(sizeof(app_timer_t));
+        app_timer_t *timer = (app_timer_t*)BH_MALLOC(sizeof(app_timer_t));
+
         if (timer == NULL)
             goto cleanup;
 
@@ -223,15 +240,18 @@ timer_ctx_t create_timer_ctx(timer_callback_f timer_handler,
         prealloc_num--;
     }
 
-    os_cond_init(&ctx->cond);
-    os_mutex_init(&ctx->mutex);
+    if (os_cond_init(&ctx->cond) != 0)
+        goto cleanup;
 
-    PRINT("timer ctx created. pre-alloc: %d\n", ctx->pre_allocated);
+    if (os_mutex_init(&ctx->mutex) != 0) {
+        os_cond_destroy(&ctx->cond);
+        goto cleanup;
+    }
 
+    PRINT("timer ctx created. pre-alloc: %d\n", ctx->pre_allocated);
     return ctx;
 
 cleanup:
-
     if (ctx) {
         release_timer_list(&ctx->free_timers);
         BH_FREE(ctx);
@@ -240,10 +260,11 @@ cleanup:
     return NULL;
 }
 
-void destroy_timer_ctx(timer_ctx_t ctx)
+void
+destroy_timer_ctx(timer_ctx_t ctx)
 {
     while (ctx->free_timers) {
-        void * tmp = ctx->free_timers;
+        void *tmp = ctx->free_timers;
         ctx->free_timers = ctx->free_timers->next;
         BH_FREE(tmp);
     }
@@ -255,12 +276,14 @@ void destroy_timer_ctx(timer_ctx_t ctx)
     BH_FREE(ctx);
 }
 
-unsigned int timer_ctx_get_owner(timer_ctx_t ctx)
+unsigned int
+timer_ctx_get_owner(timer_ctx_t ctx)
 {
     return ctx->owner;
 }
 
-void add_idle_timer(timer_ctx_t ctx, app_timer_t * timer)
+void
+add_idle_timer(timer_ctx_t ctx, app_timer_t * timer)
 {
     os_mutex_lock(&ctx->mutex);
     timer->next = ctx->idle_timers;
@@ -268,31 +291,33 @@ void add_idle_timer(timer_ctx_t ctx, app_timer_t * timer)
     os_mutex_unlock(&ctx->mutex);
 }
 
-uint32 sys_create_timer(timer_ctx_t ctx, int interval, bool is_period,
-                        bool auto_start)
+uint32
+sys_create_timer(timer_ctx_t ctx, int interval, bool is_period,
+                 bool auto_start)
 {
-
     app_timer_t *timer;
 
     if (ctx->pre_allocated) {
-        if (ctx->free_timers == NULL)
+        if (ctx->free_timers == NULL) {
             return (uint32)-1;
+        }
         else {
             timer = ctx->free_timers;
             ctx->free_timers = timer->next;
         }
-    } else {
-        timer = (app_timer_t*) BH_MALLOC(sizeof(app_timer_t));
+    }
+    else {
+        timer = (app_timer_t*)BH_MALLOC(sizeof(app_timer_t));
         if (timer == NULL)
             return (uint32)-1;
     }
 
     memset(timer, 0, sizeof(*timer));
 
-    ctx->g_max_id++;
-    if (ctx->g_max_id == (uint32)-1)
-        ctx->g_max_id++;
-    timer->id = ctx->g_max_id;
+    ctx->max_timer_id++;
+    if (ctx->max_timer_id == (uint32)-1)
+        ctx->max_timer_id++;
+    timer->id = ctx->max_timer_id;
     timer->interval = (uint32)interval;
     timer->is_periodic = is_period;
 
@@ -304,10 +329,12 @@ uint32 sys_create_timer(timer_ctx_t ctx, int interval, bool is_period,
     return timer->id;
 }
 
-bool sys_timer_cancel(timer_ctx_t ctx, uint32 timer_id)
+bool
+sys_timer_cancel(timer_ctx_t ctx, uint32 timer_id)
 {
     bool from_active;
     app_timer_t * t = remove_timer(ctx, timer_id, &from_active);
+
     if (t == NULL)
         return false;
 
@@ -317,10 +344,12 @@ bool sys_timer_cancel(timer_ctx_t ctx, uint32 timer_id)
     return from_active;
 }
 
-bool sys_timer_destroy(timer_ctx_t ctx, uint32 timer_id)
+bool
+sys_timer_destroy(timer_ctx_t ctx, uint32 timer_id)
 {
     bool from_active;
     app_timer_t * t = remove_timer(ctx, timer_id, &from_active);
+
     if (t == NULL)
         return false;
 
@@ -330,14 +359,15 @@ bool sys_timer_destroy(timer_ctx_t ctx, uint32 timer_id)
     return true;
 }
 
-bool sys_timer_restart(timer_ctx_t ctx, uint32 timer_id, int interval)
+bool
+sys_timer_restart(timer_ctx_t ctx, uint32 timer_id, int interval)
 {
     app_timer_t * t = remove_timer(ctx, timer_id, NULL);
+
     if (t == NULL)
         return false;
 
-    if (interval > 0)
-        t->interval = (uint32)interval;
+    t->interval = (uint32)interval;
 
     reschedule_timer(ctx, t);
 
@@ -346,44 +376,46 @@ bool sys_timer_restart(timer_ctx_t ctx, uint32 timer_id, int interval)
 }
 
 /*
- *
- *
  * API called by the timer manager from another thread or the kernel timer handler
- *
- *
  */
 
-// lookup the app queue by the module name
-//post a timeout message to the app queue
-//
-static void handle_expired_timers(timer_ctx_t ctx, app_timer_t * expired)
+/**
+ * lookup the app queue by the module name
+ * post a timeout message to the app queue
+ */
+static void
+handle_expired_timers(timer_ctx_t ctx, app_timer_t *expired)
 {
     while (expired) {
-        app_timer_t * t = expired;
+        app_timer_t *t = expired;
         ctx->timer_callback(t->id, ctx->owner);
 
+        /* get next expired timer first, since the following
+           operation may change expired->next */
         expired = expired->next;
         if (t->is_periodic) {
-            // if it is repeating, then reschedule it;
+            /* if it is repeating, then reschedule it; */
             reschedule_timer(ctx, t);
 
-        } else {
-            // else move it to idle list
+        }
+        else {
+            /* else move it to idle list */
             add_idle_timer(ctx, t);
         }
     }
 }
 
-int get_expiry_ms(timer_ctx_t ctx)
+uint32
+get_expiry_ms(timer_ctx_t ctx)
 {
-    int ms_to_next_expiry;
+    uint32 ms_to_next_expiry;
     uint64 now = bh_get_tick_ms();
 
     os_mutex_lock(&ctx->mutex);
-    if (ctx->g_app_timers == NULL)
-        ms_to_next_expiry = 7 * 24 * 60 * 60 * 1000; // 1 week
-    else if (ctx->g_app_timers->expiry >= now)
-        ms_to_next_expiry = (int)(ctx->g_app_timers->expiry - now);
+    if (ctx->app_timers == NULL)
+        ms_to_next_expiry = (uint32)-1;
+    else if (ctx->app_timers->expiry >= now)
+        ms_to_next_expiry = (uint32)(ctx->app_timers->expiry - now);
     else
         ms_to_next_expiry = 0;
     os_mutex_unlock(&ctx->mutex);
@@ -391,39 +423,40 @@ int get_expiry_ms(timer_ctx_t ctx)
     return ms_to_next_expiry;
 }
 
-int check_app_timers(timer_ctx_t ctx)
+int
+check_app_timers(timer_ctx_t ctx)
 {
-    os_mutex_lock(&ctx->mutex);
-
-    app_timer_t * t = ctx->g_app_timers;
-    app_timer_t * expired = NULL;
-
+    app_timer_t *t, *expired = NULL;
     uint64 now = bh_get_tick_ms();
 
+    os_mutex_lock(&ctx->mutex);
+
+    t = ctx->app_timers;
     while (t) {
         if (now >= t->expiry) {
-            ctx->g_app_timers = t->next;
+            ctx->app_timers = t->next;
 
             t->next = expired;
             expired = t;
 
-            t = ctx->g_app_timers;
-        } else {
+            t = ctx->app_timers;
+        }
+        else {
             break;
         }
     }
     os_mutex_unlock(&ctx->mutex);
 
     handle_expired_timers(ctx, expired);
-
     return get_expiry_ms(ctx);
 }
 
-void cleanup_app_timers(timer_ctx_t ctx)
+void
+cleanup_app_timers(timer_ctx_t ctx)
 {
     os_mutex_lock(&ctx->mutex);
 
-    release_timer_list(&ctx->g_app_timers);
+    release_timer_list(&ctx->app_timers);
     release_timer_list(&ctx->idle_timers);
 
     os_mutex_unlock(&ctx->mutex);

core/shared/utils/runtime_timer.h

@@ -33,7 +33,7 @@ bool sys_timer_cancel(timer_ctx_t ctx, uint32 timer_id);
 bool sys_timer_restart(timer_ctx_t ctx, uint32 timer_id, int interval);
 void cleanup_app_timers(timer_ctx_t ctx);
 int check_app_timers(timer_ctx_t ctx);
-int get_expiry_ms(timer_ctx_t ctx);
+uint32 get_expiry_ms(timer_ctx_t ctx);
 
 #ifdef __cplusplus
 }