--- a/src/irc_log.c	Wed Apr 01 19:31:11 2009 +0300
+++ b/src/irc_log.c	Thu Apr 02 02:25:35 2009 +0300
@@ -27,6 +27,9 @@
     
     /** Are we currently unloading ourself (via irc_log_unload) ? */
     bool unloading;
+
+    /** The `struct module` passed to us once we unload() */
+    struct module *module;
 };
 
 /**
@@ -53,22 +56,13 @@
 
 /**
  * The irc_log_ctx has completed stopping all the channels, and should now destroy itself
- *
  */
 static void irc_log_stopped (struct irc_log_ctx *ctx)
 {
-    log_debug("destroying the irc_log_ctx");
-
-    // schedule an evsql_destroy
-    if (evsql_destroy_next(ctx->db))
-        log_fatal("evsql_destroy_next failed");
+    log_debug("module unload() completed");
     
-    // hands-off on the db
-    ctx->db = NULL;
-
-    // free ourself
-    // XXX: wrong, we need to implement a irc_log_destroy that's called by module?
-    free(ctx);
+    // notify
+    module_unloaded(ctx->module);
 }
 
 /**
@@ -478,13 +472,14 @@
 /**
  * Deinitialize, logging CLOSE events for all channels, and removing any hooks we've added
  */
-static err_t irc_log_unload (void *_ctx)
+static err_t irc_log_unload (void *_ctx, struct module *module)
 {
     struct irc_log_ctx *ctx = _ctx;
     struct irc_log_chan *chan_ctx;
 
     // update our state to mark ourselves as unloading
     ctx->unloading = true;
+    ctx->module = module;
 
     // stop logging each channel
     TAILQ_FOREACH(chan_ctx, &ctx->channels, ctx_channels) {
@@ -496,11 +491,28 @@
 }
 
 /**
+ * We can safely destroy the evsql instance from here, since we're outside of any callbacks from this module
+ */
+static void irc_log_destroy (void *_ctx)
+{
+    struct irc_log_ctx *ctx = _ctx;
+
+    log_debug("destroying the irc_log_ctx");
+
+    // destroy the evsql instance
+    evsql_destroy(ctx->db);
+
+    // ...no more
+    free(ctx);
+}
+
+/**
  * The module function table
  */
 struct module_desc irc_log_module = {
     .init               = &irc_log_init,
     .config_options     = irc_log_config_options,
     .unload             = &irc_log_unload,
+    .destroy            = &irc_log_destroy
 };
 

--- a/src/module.c	Wed Apr 01 19:31:11 2009 +0300
+++ b/src/module.c	Thu Apr 02 02:25:35 2009 +0300
@@ -38,6 +38,68 @@
     return old_path;
 }
 
+/**
+ * module_get without the refcount stuff
+ */
+static struct module* module_lookup (struct modules *modules, const char *name)
+{
+    struct module *module = NULL;
+
+    // look for it...
+    TAILQ_FOREACH(module, &modules->list, modules_list) {
+        if (strcasecmp(module->info.name, name) == 0) {
+            // found it
+            return module;
+        }
+    }
+
+    // no such module
+    return NULL;
+}
+
+struct module* module_get (struct modules *modules, const char *name)
+{
+    struct module *module;
+    
+    // get it
+    if (!(module = module_lookup(modules, name)))
+        return NULL;
+
+    // up the refcount
+    module->refcount++;
+    
+    // ok
+    return module;
+}
+
+err_t modules_unload (struct modules *modules)
+{
+    struct module *module;
+    err_t err;
+
+    // unload each module in turn
+    while ((module = TAILQ_FIRST(&modules->list))) {
+        if ((err = module_unload(module)))
+            log_warn("module_unload(%s) failed: %s", module->info.name, error_name(err));
+    }
+
+    // ok
+    return SUCCESS;
+}
+
+void modules_destroy (struct modules *modules)
+{
+    struct module *module;
+
+    // destroy each module
+    while ((module = TAILQ_FIRST(&modules->list)))
+        module_destroy(module);
+
+    // ourselves
+    free(modules);
+}
+
+
 const char* module_name (struct module *module)
 {
     return module->info.name;
@@ -131,16 +193,17 @@
         return SET_ERROR(err, ERR_MODULE_NAME);
     
     // already open with the same name?
-    if (module_get(modules, _info->name))
+    if (module_lookup(modules, _info->name))
         return SET_ERROR(err, ERR_MODULE_DUP);
 
     // alloc
     if ((module = calloc(1, sizeof(*module))) == NULL)
         return SET_ERROR(err, ERR_CALLOC);
     
-    // store
+    // initialize
     module->info = *_info;
     module->modules = modules;
+    module->refcount = 1;
  
     // add to modules list
     TAILQ_INSERT_TAIL(&modules->list, module, modules_list);
@@ -182,28 +245,27 @@
 
 error:
     // cleanup
+    module->refcount = 0;
     module_destroy(module);
 
     return ERROR_CODE(err);    
 }
 
-struct module* module_get (struct modules *modules, const char *name)
+void module_put (struct module *module)
 {
-    struct module *module = NULL;
-
-    // look for it...
-    TAILQ_FOREACH(module, &modules->list, modules_list) {
-        if (strcasecmp(module->info.name, name) == 0)
-            // found it
-            return module;
-    }
-
-    // no such module
-    return NULL;
+    assert(module->refcount > 0);
+    
+    // decrement, just return if still alive
+    if (--module->refcount > 0)
+        return;
+    
+    // refcount zero, destroy
+    module_destroy(module);
 }
 
 err_t module_conf_raw (struct module *module, const char *name, char *value, struct error_info *err)
 {
+    // sanity-check
     if (!module->desc->config_options)
         RETURN_SET_ERROR_STR(err, ERR_MODULE_CONF, "module does not have any config options");
 
@@ -217,6 +279,7 @@
 
 const struct config_option* module_conf_lookup (struct module *module, const char *name, struct error_info *err)
 {
+    // sanity-check
     if (!module->desc->config_options)
         JUMP_SET_ERROR_STR(err, ERR_MODULE_CONF, "module does not have any config options");
     
@@ -240,23 +303,69 @@
 err_t module_unload (struct module *module)
 {
     err_t err;
+    
+    // sanity-check
+    assert(!module->unloading);
 
-    // invoke the unload func
-    if ((err = module->desc->unload(module->ctx)))
+    // remove from modules list
+    TAILQ_REMOVE(&module->modules->list, module, modules_list);
+    
+    // update status
+    module->unloading = true;
+
+    // invoke the unload func, passing it our reference
+    // note that the module might not exist anymore after calling this
+    if ((err = module->desc->unload(module->ctx, module)))
         return err;
-        
-    // update status
-    // XXX: before or after?
-    module->unloading = true;
 
     // ok
     return SUCCESS;
 }
 
+/**
+ * A wrapper for module_destroy suitable for use as a libevent callback
+ */
+static void _module_destroy (int fd, short what, void *arg)
+{
+    struct module *module = arg;
+
+    (void) fd;
+    (void) what;
+    
+    // execute
+    module_destroy(module);
+}
+
+void module_unloaded (struct module *module)
+{
+    struct timeval tv = { 0, 0 };
+    assert(module->refcount > 0);
+    
+    // decrement, just return if still alive
+    if (--module->refcount > 0)
+        return;
+    
+    // schedule a deferred module_destroy
+    if (event_base_once(module->modules->nexus->ev_base, -1, EV_TIMEOUT, &_module_destroy, module, &tv))
+        // XXX: how to reach?
+        log_fatal("event_base_once failed, unable to schedule deferred module_destroy");
+}
+
 void module_destroy (struct module *module)
 {
-    // remove from modules list
-    TAILQ_REMOVE(&module->modules->list, module, modules_list);
+    // XXX: warn about destroy with significant refcount...
+    if (module->refcount)
+        log_warn("destroying module %s with refcount>0: %zu", module_name(module), module->refcount);
+    else
+        log_debug("destroying module %s", module_name(module));
+
+    // still need to remove from modules_list?
+    if (!module->unloading)
+        TAILQ_REMOVE(&module->modules->list, module, modules_list);
+
+    // run the destroy hook
+    if (module->desc->destroy)
+        module->desc->destroy(module->ctx);
 
     // unload the dl handle
     if (module->handle && dlclose(module->handle))
@@ -269,30 +378,4 @@
     free(module);
 }
 
-err_t modules_unload (struct modules *modules)
-{
-    struct module *module;
-    err_t err;
-
-    // unload each module in turn
-    TAILQ_FOREACH(module, &modules->list, modules_list) {
-        if ((err = module_unload(module)))
-            log_warn("module_unload(%s) failed: %s", module->info.name, error_name(err));
-    }
 
-    // ok
-    return SUCCESS;
-}
-
-void modules_destroy (struct modules *modules)
-{
-    struct module *module;
-
-    // destroy each module
-    while ((module = TAILQ_FIRST(&modules->list)))
-        module_destroy(module);
-
-    // ourselves
-    free(modules);
-}
-

--- a/src/module.h	Wed Apr 01 19:31:11 2009 +0300
+++ b/src/module.h	Thu Apr 02 02:25:35 2009 +0300
@@ -9,7 +9,27 @@
  * The modules are loaded using dlopen(), and hence should be standard dynamic libraries. Modules are then "loaded" by
  * resolving a `struct module_funcs` symbol called "<modname>_funcs", and then using the init func to construct a
  * "context", which is then further manipulated by various other module functions.
+ *
+ * Modules are also reference counted, mainly for implementing module_unload(). When a module is first loaded, it has
+ * a reference count of one - the entry in struct modules. Later, modules can be referenced using module_get(), and
+ * returned with module_put() once done. The module should never be destroyed during its lifetime, until a
+ * module_unload() occurs. At this point, the module is removed from the modules_list, and the one reference is
+ * 'passed on' to the module itself. Once it has finished unloading, it can call module_unloaded() on the reference it was
+ * given by module_desc::unload, which may then result in a deferred module_destroy().
+ *
+ * Module destroying itself is an interesting issue, since modules effectively need to be able to destroy themselves
+ * (as they must be able to perform cleanup, and then notify completion from inside an event loop callback). This means
+ * that they cannot directly execute a module_destroy() on themselves - if we call dlclose() with dlopen-mapped code
+ * pages on the stack, a segfault ensues. Hence, they must call module_unloaded() on themselves, which then executes a
+ * deferred module_destroy() if there are no references left. Otherwise, the module should be safe from external code,
+ * as module_put() should never cause a module to be destroyed before module_unloaded() is executed, due to the primary
+ * reference.
+ *
+ * Ugh, it's compliated, I need to write a clearer explenation once it's implemented :)
  */
+
+struct module;
+
 #include "nexus.h"
 #include "config.h"
 #include "error.h"
@@ -37,6 +57,8 @@
      * Initialize the module, returning an opaque context pointer that is stored in the module state, and supplied for
      * subsequent calls. The supplied nexus arg can be used to access the global state.
      *
+     * Implementing this is mandatory.
+     *
      * @param nexus a pointer to the nexus struct containing the global state
      * @param ctx_ptr the context pointer should be returned via this
      * @param err returned error info
@@ -48,6 +70,8 @@
      * array of config_option's.
      *
      * The handler functions will recieve the module's context pointer as their ctx argument.
+     *
+     * Implementing this is optional, but recommended.
      */
     const struct config_option *config_options;
 
@@ -57,9 +81,26 @@
      * cleanly. But the unloading should complete reasonably quickly, after which all event handlers added by the
      * module to the nexus' ev_base should have been removed, and resources released.
      *
+     * The module given as an argument is the module itself - which has been removed from the modules_list - the
+     * primary reference is passed on to the module. Once the module has finished unloading, it may call module_put(),
+     * which may then call module_destroy(), if no other references were left.
+     *
+     * Implementing this is mandatory.
+     *
      * @param ctx the module's context pointer as returned by init
      */
-    err_t (*unload) (void *ctx);
+    err_t (*unload) (void *ctx, struct module *module);
+
+    /**
+     * Destroy the module now. No later chances, the module's code will be unloaded directly after this, which means
+     * that attempts to execute the module's code (even on the stack...) after this will segfault.
+     *
+     * The module code /should/ garuntee that this is never called from *inside* the module code - calls to
+     * module_destroy() will be deferred via the event loop if needed.
+     *
+     * Implementing this is optional.
+     */
+    void (*destroy) (void *ctx);
 };
 
 /**
@@ -84,9 +125,10 @@
     /** Reference back to modules struct used to load this module */
     struct modules *modules;
 
-    /**
-     * Is the module currently being unloaded?
-     */
+    /** Reference count for destroy() */
+    size_t refcount;
+    
+    /** Is the module currently being unloaded? */
     bool unloading;
 
     /** Our entry in the list of modules */
@@ -157,6 +199,30 @@
 const char* modules_path (struct modules *modules, const char *path);
 
 /**
+ * Get a reference to the module, which must be returned using module_put
+ *
+ * @param modules the modules state
+ * @param name the module name to get
+ * @return the module struct, or NULL if not found
+ */
+struct module* module_get (struct modules *modules, const char *name);
+
+/**
+ * Unload all modules, this just calls module_unload for each module, logging errors as warnings.
+ */
+err_t modules_unload (struct modules *modules);
+
+/**
+ * Destroy all modules immediately.
+ */
+void modules_destroy (struct modules *modules);
+
+
+
+/*******************************************/
+
+
+/**
  * Return a module's name
  */
 const char* module_name (struct module *module);
@@ -174,13 +240,9 @@
 err_t module_load (struct modules *modules, struct module **module_ptr, const struct module_info *info, struct error_info *err);
 
 /**
- * Lookup a module by name
- *
- * @param modules the modules state
- * @param name the module name to get
- * @return the module struct, or NULL if not found
+ * Return a module retrieved using module_get
  */
-struct module* module_get (struct modules *modules, const char *name);
+void module_put (struct module *module);
 
 /**
  * Look up a module configuration option by name
@@ -198,30 +260,26 @@
 err_t module_conf_raw (struct module *module, const char *name, char *value, struct error_info *err);
 
 /**
- * Unload a module. This means calling its 'unload' func, which will then go about shutting down the module. This might
- * not happen right away, though, so the module is not destroyed yet.
- *
- * XXX: currently the module is never destroyed, there needs to be a "module unload done" callback...
+ * Unload a module. This removes the module from the modules_list, marks it as unloading, and then calls the module's
+ * \p unload function, passing it the primary reference. The module's unload code will then go about shutting down the
+ * module, and once that is done, it may module_put() the primary reference, which may then lead to module_destroy().
  */
 err_t module_unload (struct module *module);
 
 /**
- * Destroy a module, releasing as many resources as possible
+ * Used by a module itself to indicate that an module_desc::unload() operation has completed. This will execute a
+ * deferred module_destroy() if there are no more references left on the module.
+ *
+ * Note: this is not intended to be called from outside the given module itself...
+ */
+void module_unloaded (struct module *module);
+
+/**
+ * Destroy a module, releasing as many resources as possible, but not stopping for errors.
+ *
+ * This does not enforce the correct refcount - 'tis the caller's responsibility. Prints out a warning if
+ * refcount > 0.
  */
 void module_destroy (struct module *module);
 
-/**
- * Unload all modules, this just calls module_unload for each module, logging errors as warnings.
- *
- * XXX: currently, this does not destroy any modules, or the modules state itself.
- */
-err_t modules_unload (struct modules *modules);
-
-/**
- * Destroy all modules immediately.
- *
- * XXX: this doesn't leaves hanging module resources. The program will probably crash in soon
- */
-void modules_destroy (struct modules *modules);
-
 #endif

--- a/src/nexus.c	Wed Apr 01 19:31:11 2009 +0300
+++ b/src/nexus.c	Thu Apr 02 02:25:35 2009 +0300
@@ -441,8 +441,13 @@
     // remove the signal handlers
     if (nexus->signals)
         signals_free(nexus->signals);
+    
+    // finally, the libevent base
+    if (nexus->ev_base)
+        event_base_free(nexus->ev_base);
 
-    // ok, nexus is now dead
+    // ok, nexus is now dead, so drop all pointers to make valgrind show things as leaked :)
+    memset(nexus, 0, sizeof(*nexus));
 }
 
 static void on_sigint (evutil_socket_t sig, short what, void *arg)

--- a/test/valgrind-suppressions	Wed Apr 01 19:31:11 2009 +0300
+++ b/test/valgrind-suppressions	Thu Apr 02 02:25:35 2009 +0300
@@ -125,3 +125,16 @@
     fun:module_load
 }
 
+# this is an allocate-global-state-on-the-heap "bug", see:
+#   http://sourceware.org/bugzilla/show_bug.cgi?id=2314
+{
+    getpwuid_r-1
+    Memcheck:Leak
+    fun:malloc
+    obj:/lib/libc-2.7.so
+    fun:__nss_database_lookup
+    obj:*
+    obj:*
+    fun:getpwuid_r
+}
+