Libmodule 4.2.0 with a small behavioral change: avoid starting module…

… as soon as they're registered. Start them (and call their init() callback) just once ctx starts looping. Some small refactoring too.

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required (VERSION 3.3.2)
 
-project(libmodule VERSION 4.1.0 LANGUAGES C CXX)
+project(libmodule VERSION 4.2.0 LANGUAGES C CXX)
 
 if(NOT CMAKE_BUILD_TYPE)
    set(CMAKE_BUILD_TYPE Release)

Lib/module.c

@@ -104,7 +104,6 @@ static module_ret_code _register_fd(module *mod, const int fd, const bool autocl
         tmp->prev = mod->fds;
         tmp->self = &mod->self;
         mod->fds = tmp;
-        mod->self.ctx->num_fds++;
         /* If a fd is registered at runtime, start polling on it */
         int ret = 0;
         if (_module_is(mod, RUNNING)) {
@@ -116,7 +115,7 @@ static module_ret_code _register_fd(module *mod, const int fd, const bool autocl
     return MOD_ERR;
 }
 
-/* Linearly searching for fd */
+/* Linear search for fd */
 static module_ret_code _deregister_fd(module *mod, const int fd) {
     module_poll_t **tmp = &mod->fds;
 
@@ -134,7 +133,6 @@ static module_ret_code _deregister_fd(module *mod, const int fd) {
             }
             memhook._free(t->ev);
             memhook._free(t);
-            mod->self.ctx->num_fds--;
             return !ret ? MOD_OK : MOD_ERR;
         }
         tmp = &(*tmp)->prev;
@@ -205,12 +203,12 @@ bool _module_is(const module *mod, const enum module_states st) {
     return mod->state & st;
 }
 
-
 int evaluate_module(void *data, void *m) {
     module *mod = (module *)m;
     if (_module_is(mod, IDLE) && mod->hook.evaluate()) {
-        mod->hook.init();
-        start(mod, "Failed to start module.");
+        if (start(mod, "Failed to start module.") == MOD_OK) {
+            mod->hook.init();
+        }
     }
     return MAP_OK;
 }
@@ -272,7 +270,8 @@ module_ret_code module_register(const char *name, const char *ctx_name, const se
         memcpy((self_t *)&mod->self, &((self_t){ mod, context, true }), sizeof(self_t));
 
         if (map_put(context->modules, mod->name, mod, false, false) == MAP_OK) {
-            evaluate_module(NULL, mod);
+            mod->pubsub_fd[0] = -1;
+            mod->pubsub_fd[1] = -1;
             return MOD_OK;
         }
         ret = MOD_ERR;
@@ -296,7 +295,7 @@ module_ret_code module_deregister(const self_t **self) {
     /* Free all unread pubsub msg for this module */
     flush_pubsub_msg(tmp, mod);
 
-    stop(mod, "Failed to stop module.", 1);
+    stop(mod, "Failed to stop module.", true);
 
     /* Remove the module from the context */
     map_remove(tmp->ctx->modules, mod->name);

Lib/modules.c

@@ -5,8 +5,9 @@ static _ctor1_ void modules_init(void);
 static _dtor0_ void modules_destroy(void);
 static void evaluate_new_state(m_context *c);
 static module_ret_code loop_start(m_context *c, const int max_events);
-static int recv_events(m_context *c, int timeout);
 static int loop_stop(m_context *c);
+static inline int loop_quit(m_context *c, const uint8_t quit_code);
+static int recv_events(m_context *c, int timeout);
 
 map_t *ctx;
 memalloc_hook memhook;
@@ -31,14 +32,36 @@ static module_ret_code loop_start(m_context *c, const int max_events) {
         c->looping = true;
         c->quit_code = 0;
         c->max_events = max_events;
-
-        /* Tell every module that loop is started */
+
+        /* Eventually start any IDLE module */
+        evaluate_new_state(c);
+
+        /* Tell every RUNNING module that loop is started */
         tell_system_pubsub_msg(c, LOOP_STARTED, NULL);
         return MOD_OK;
     }
     return MOD_ERR;
 }
 
+static int loop_stop(m_context *c) {
+    /* Tell every module that loop is stopped */
+    tell_system_pubsub_msg(c, LOOP_STOPPED, NULL);
+
+    /* Flush pubsub msg to avoid memleaks */
+    map_iterate(c->modules, flush_pubsub_msg, NULL);
+
+    poll_destroy_pevents(&c->pevents, &c->max_events);
+    c->looping = false;
+    c->quit = false;
+    return c->quit_code;
+}
+
+static inline int loop_quit(m_context *c, const uint8_t quit_code) {
+    c->quit = true;
+    c->quit_code = quit_code;
+    return MOD_OK;
+}
+
 static int recv_events(m_context *c, int timeout) {
     int nfds = poll_wait(c->fd, c->max_events, c->pevents, timeout);
     for (int i = 0; i < nfds; i++) {
@@ -84,28 +107,14 @@ static int recv_events(m_context *c, int timeout) {
     } else if (nfds < 0) {
         if (errno != EINTR && errno != EAGAIN) {
             fprintf(stderr, "Module loop error: %s.\n", strerror(errno));
-            c->quit = true;
-            c->quit_code = errno;
+            loop_quit(c, errno);
         } else {
             nfds = 0; // return < 0 only for real errors
         }
     }
     return nfds;
 }
 
-static int loop_stop(m_context *c) {
-    /* Tell every module that loop is stopped */
-    tell_system_pubsub_msg(c, LOOP_STOPPED, NULL);
-
-    /* Flush pubsub msg to avoid memleaks */
-    map_iterate(c->modules, flush_pubsub_msg, NULL);
-
-    poll_destroy_pevents(&c->pevents, &c->max_events);
-    c->looping = false;
-    c->quit = false;
-    return c->quit_code;
-}
-
 /** Public API **/
 
 module_ret_code modules_set_memalloc_hook(const memalloc_hook *hook) {
@@ -135,7 +144,6 @@ module_ret_code modules_ctx_set_logger(const char *ctx_name, const log_cb logger
 module_ret_code modules_ctx_loop_events(const char *ctx_name, const int max_events) {
     MOD_PARAM_ASSERT(max_events > 0);
     FIND_CTX(ctx_name);
-    MOD_ASSERT(c->num_fds > 0, "No fds to loop on.", MOD_ERR);
     MOD_ASSERT(!c->looping, "Context already looping.", MOD_ERR);
 
     if (loop_start(c, max_events) == MOD_OK) {
@@ -151,9 +159,7 @@ module_ret_code modules_ctx_quit(const char *ctx_name, const uint8_t quit_code)
     FIND_CTX(ctx_name);
     MOD_ASSERT(c->looping, "Context not looping.", MOD_ERR);
 
-    c->quit = true;
-    c->quit_code = quit_code;
-    return MOD_OK;
+    return loop_quit(c, quit_code);
 }
 
 module_ret_code modules_ctx_get_fd(const char *ctx_name, int *fd) {

Lib/priv.h

@@ -103,7 +103,6 @@ struct _context {
     uint8_t quit_code;
     bool looping;
     int fd;
-    int num_fds;
     log_cb logger;
     map_t *modules;
     void *pevents;

Lib/pubsub.c

@@ -82,7 +82,7 @@ module_ret_code tell_system_pubsub_msg(m_context *c, enum msg_type type, const c
 int flush_pubsub_msg(void *data, void *m) {
     module *mod = (module *)m;
     pubsub_msg_t *mm = NULL;
-    
+
     while (read(mod->pubsub_fd[0], &mm, sizeof(pubsub_msg_t *)) == sizeof(pubsub_msg_t *)) {
         /* 
          * Actually tell msg ONLY if we are not deregistering module, 

Samples/MultiCtx/a.c

@@ -36,7 +36,7 @@ static void init(void) {
     timerValue.it_value.tv_sec = 1;
     timerValue.it_interval.tv_sec = 1;
     timerfd_settime(fd, 0, &timerValue, NULL);
-    m_register_fd(fd, 1, NULL);
+    m_register_fd(fd, true, NULL);
 #endif
 }
 

Samples/MultiCtx/b.c

@@ -3,6 +3,7 @@
 #ifdef __linux__
     #include <sys/signalfd.h>
     #include <signal.h>
+    #include <bits/sigaction.h>
 #endif
 #include <unistd.h>
 
@@ -28,14 +29,12 @@ static void module_pre_start(void) {
 static void init(void) {
 #ifdef __linux__
     sigset_t mask;
-
     sigemptyset(&mask);
     sigaddset(&mask, SIGINT);
     sigaddset(&mask, SIGTERM);
-    sigprocmask(SIG_BLOCK, &mask, NULL);
 
-    int fd = signalfd(-1, &mask, 0);
-    m_register_fd(fd, 1, NULL);
+    int fd = signalfd(-1, &mask, SFD_NONBLOCK | SFD_CLOEXEC);
+    m_register_fd(fd, true, NULL);
 #endif
 }
 
@@ -63,7 +62,7 @@ static bool evaluate(void) {
 
 /*
  * Destroyer function, called at module unload (at end of program).
- * Note that module's FD is automatically closed for you.
+ * Note that module's FD is automatically closed for you if autoclose is set when registering it.
  */
 static void destroy(void) {
 

Samples/MultiCtx/main.c

@@ -2,6 +2,10 @@
 #include <module/module.h>
 #include <pthread.h>
 #include <stdlib.h>
+#ifdef __linux__
+    #include <signal.h>
+    #include <bits/sigaction.h>
+#endif
 
 static void *loop(void *param);
 
@@ -17,7 +21,20 @@ static void logger(const self_t *self, const char *fmt, va_list args, const void
     }
 }
 
+void setup_signals(void) {
+#ifdef __linux__
+    sigset_t mask;
+    sigemptyset(&mask);
+    sigaddset(&mask, SIGINT);
+    sigaddset(&mask, SIGTERM);
+    sigprocmask(SIG_BLOCK, &mask, NULL);
+#endif
+}
+
 int main() {
+    /* Properly block signals (that are received by mod "B" on ctx2) */
+    setup_signals();
+
     /* Our 2 contexts names */
     char *ctx1 = "FirstCtx";
     char *ctx2 = "SecondCtx";

TODO.md

@@ -1,5 +1,15 @@
 ## 4.2.0
 
+### Generic 
+- [x] Avoid calling evaluate_module when registering a new module. Call evaluate_module on each module when loop_start() is called.
+- [x] Fix default values for pubsub_fd[] for each module
+- [x] Update tests (module_start should now be called upon registering)
+- [x] Fix MultiCtx sample (signalfd multithread crazyness)
+- [x] Update doc (changed behaviours!)
+- [x] Drop mcontext -> num_fds
+
+## 4.3.0
+
 ### Submodules
 - [ ] SUBMODULE(B, A) calls module_register(B) and module_binds_to(A);
 - [ ] SUBMODULE SHOULD BE STARTED later (after all MODULES) -> ctor3 (this way it'll support only level 1 submodules though (ie: a submodule cannot have submodules))

docs/conf.py

@@ -23,8 +23,8 @@
 copyright = '2019, Federico Di Pierro'
 author = 'Federico Di Pierro'
 
-version = '4.1.0'
-release = '4.1.0'
+version = '4.2.0'
+release = '4.2.0'
 
 highlight_language = "c"
 

docs/src/lifecycle.rst

@@ -22,8 +22,8 @@ For every module, check() function will be called; if and only if that function
 An unregistered module is just dead code; none of its functions will ever be called. |br|
 Once a module is registered, it will be initially set to an IDLE state. Idle means that the module is not started yet, thus it cannot receive any PubSub msg nor any event from fds. |br|
 
-Right after module's registration, its evaluate() function will be called, trying to start the module right after it was registered. |br|
-Evaluate will be then called at each state machine change, for each idle module. |br|
+As soon as its context starts looping, a module's evaluate() function will be called, trying to start it right away. |br|
+Evaluate() will be then called at each state machine change, for each idle module. |br|
 
 As soon as module's evaluate() returns true, the module is started. It means its init() function is finally called and its state is set to RUNNING. |br|
 A single module can poll on multiple fds: just call module_register_fd() multiple times. |br|

tests/main.c

@@ -36,6 +36,8 @@ int main(void) {
         cmocka_unit_test(test_modules_ctx_quit_no_loop),
 
         /* Test module state setters */
+        cmocka_unit_test(test_module_start_NULL_self),
+        cmocka_unit_test(test_module_start),
         cmocka_unit_test(test_module_pause_NULL_self),
         cmocka_unit_test(test_module_pause),
         cmocka_unit_test(test_module_resume_NULL_self),

tests/test_module.c

@@ -57,7 +57,7 @@ void test_module_register(void **state) {
     module_ret_code ret = module_register("testName", CTX, &self, &hook);
     assert_true(ret == MOD_OK);
     assert_non_null(self);
-    assert_true(module_is(self, RUNNING));
+    assert_true(module_is(self, IDLE));
 }
 
 void test_module_register_already_registered(void **state) {
@@ -67,7 +67,7 @@ void test_module_register_already_registered(void **state) {
     module_ret_code ret = module_register("testName", CTX, &self, &hook);
     assert_false(ret == MOD_OK);
     assert_non_null(self);
-    assert_true(module_is(self, RUNNING));
+    assert_true(module_is(self, IDLE));
 }
 
 void test_module_register_same_name(void **state) {