Use a separate interface for loggers.
authorTobias Brunner <tobias@strongswan.org>
Sat, 21 Jan 2012 13:47:13 +0000 (14:47 +0100)
committerTobias Brunner <tobias@strongswan.org>
Wed, 2 May 2012 12:45:38 +0000 (14:45 +0200)
The new interface does not allow loggers to unregister themselves from
the bus.  This allows us to use a rwlock_t for them.

The latter also means that loggers can now be called concurrently by
multiple threads.

17 files changed:
src/charon/charon.c
src/libcharon/Makefile.am
src/libcharon/bus/bus.c
src/libcharon/bus/bus.h
src/libcharon/bus/listeners/file_logger.c
src/libcharon/bus/listeners/file_logger.h
src/libcharon/bus/listeners/listener.h
src/libcharon/bus/listeners/logger.h [new file with mode: 0644]
src/libcharon/bus/listeners/sys_logger.c
src/libcharon/bus/listeners/sys_logger.h
src/libcharon/control/controller.c
src/libcharon/plugins/android/android_logger.c
src/libcharon/plugins/android/android_logger.h
src/libcharon/plugins/android/android_plugin.c
src/libcharon/plugins/sql/sql_logger.c
src/libcharon/plugins/sql/sql_logger.h
src/libcharon/plugins/sql/sql_plugin.c

index 6dbb0b5..5e55221 100644 (file)
@@ -335,7 +335,7 @@ static void initialize_loggers(bool use_stderr, level_t levels[])
                                                                           facility, debug_lower_names, group));
                }
                charon->sys_loggers->insert_last(charon->sys_loggers, sys_logger);
-               charon->bus->add_listener(charon->bus, &sys_logger->listener);
+               charon->bus->add_logger(charon->bus, &sys_logger->logger);
        }
        enumerator->destroy(enumerator);
 
@@ -385,7 +385,7 @@ static void initialize_loggers(bool use_stderr, level_t levels[])
                                                                           filename, debug_lower_names, group));
                }
                charon->file_loggers->insert_last(charon->file_loggers, file_logger);
-               charon->bus->add_listener(charon->bus, &file_logger->listener);
+               charon->bus->add_logger(charon->bus, &file_logger->logger);
 
        }
        enumerator->destroy(enumerator);
@@ -395,11 +395,11 @@ static void initialize_loggers(bool use_stderr, level_t levels[])
        {
                /* set up default stdout file_logger */
                file_logger = file_logger_create(stdout, NULL, FALSE);
-               charon->bus->add_listener(charon->bus, &file_logger->listener);
+               charon->bus->add_logger(charon->bus, &file_logger->logger);
                charon->file_loggers->insert_last(charon->file_loggers, file_logger);
                /* set up default daemon sys_logger */
                sys_logger = sys_logger_create(LOG_DAEMON, FALSE);
-               charon->bus->add_listener(charon->bus, &sys_logger->listener);
+               charon->bus->add_logger(charon->bus, &sys_logger->logger);
                charon->sys_loggers->insert_last(charon->sys_loggers, sys_logger);
                for (group = 0; group < DBG_MAX; group++)
                {
@@ -412,7 +412,7 @@ static void initialize_loggers(bool use_stderr, level_t levels[])
 
                /* set up default auth sys_logger */
                sys_logger = sys_logger_create(LOG_AUTHPRIV, FALSE);
-               charon->bus->add_listener(charon->bus, &sys_logger->listener);
+               charon->bus->add_logger(charon->bus, &sys_logger->logger);
                charon->sys_loggers->insert_last(charon->sys_loggers, sys_logger);
                sys_logger->set_level(sys_logger, DBG_ANY, LEVEL_AUDIT);
        }
index a322b0c..f979843 100755 (executable)
@@ -3,6 +3,7 @@ ipseclib_LTLIBRARIES = libcharon.la
 libcharon_la_SOURCES = \
 bus/bus.c bus/bus.h \
 bus/listeners/listener.h \
+bus/listeners/logger.h \
 bus/listeners/file_logger.c bus/listeners/file_logger.h \
 bus/listeners/sys_logger.c bus/listeners/sys_logger.h \
 config/backend_manager.c config/backend_manager.h config/backend.h \
index df15afb..4f1a4ce 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011 Tobias Brunner
+ * Copyright (C) 2011-2012 Tobias Brunner
  * Copyright (C) 2006 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
@@ -21,6 +21,7 @@
 #include <threading/thread.h>
 #include <threading/thread_value.h>
 #include <threading/mutex.h>
+#include <threading/rwlock.h>
 
 typedef struct private_bus_t private_bus_t;
 
@@ -34,24 +35,24 @@ struct private_bus_t {
        bus_t public;
 
        /**
-        * List of registered listeners as entry_t's
+        * List of registered listeners as entry_t.
         */
        linked_list_t *listeners;
 
        /**
-        * List of registered listeners implementing listener_t.log() as entry_t
+        * List of registered loggers.
         */
        linked_list_t *loggers;
 
        /**
-        * mutex for the list of listeners, recursively
+        * Mutex for the list of listeners, recursively.
         */
        mutex_t *mutex;
 
        /**
-        * mutex for the list of loggers
+        * Read-write lock for the list of loggers.
         */
-       mutex_t *log_mutex;
+       rwlock_t *log_lock;
 
        /**
         * Thread local storage the threads IKE_SA
@@ -76,61 +77,27 @@ struct entry_t {
         */
        int calling;
 
-       /**
-        * are we currently logging on this listener
-        */
-       int logging;
-
-       /**
-        * TRUE if this listener is active (we use this for the loggers)
-        */
-       bool enabled;
 };
 
-/**
- * create a listener entry
- */
-static entry_t *entry_create(listener_t *listener)
+METHOD(bus_t, add_listener, void,
+       private_bus_t *this, listener_t *listener)
 {
-       entry_t *this;
+       entry_t *entry;
 
-       INIT(this,
+       INIT(entry,
                .listener = listener,
-               .enabled = TRUE,
        );
-       return this;
-}
-
-/**
- * destroy an entry_t
- */
-static void entry_destroy(entry_t *entry)
-{
-       free(entry);
-}
-
-METHOD(bus_t, add_listener, void,
-       private_bus_t *this, listener_t *listener)
-{
-       entry_t *entry = entry_create(listener);
 
        this->mutex->lock(this->mutex);
        this->listeners->insert_last(this->listeners, entry);
        this->mutex->unlock(this->mutex);
-
-       if (listener->log)
-       {
-               this->log_mutex->lock(this->log_mutex);
-               this->loggers->insert_last(this->loggers, entry);
-               this->log_mutex->unlock(this->log_mutex);
-       }
 }
 
 METHOD(bus_t, remove_listener, void,
        private_bus_t *this, listener_t *listener)
 {
        enumerator_t *enumerator;
-       entry_t *entry, *found = NULL;
+       entry_t *entry;
 
        this->mutex->lock(this->mutex);
        enumerator = this->listeners->create_enumerator(this->listeners);
@@ -139,33 +106,29 @@ METHOD(bus_t, remove_listener, void,
                if (entry->listener == listener)
                {
                        this->listeners->remove_at(this->listeners, enumerator);
-                       found = entry;
+                       free(entry);
                        break;
                }
        }
        enumerator->destroy(enumerator);
        this->mutex->unlock(this->mutex);
-
-       if (found)
-       {
-               this->log_mutex->lock(this->log_mutex);
-               this->loggers->remove(this->loggers, found, NULL);
-               this->log_mutex->unlock(this->log_mutex);
-               entry_destroy(found);
-       }
 }
 
-typedef struct cleanup_data_t cleanup_data_t;
+METHOD(bus_t, add_logger, void,
+       private_bus_t *this, logger_t *logger)
+{
+       this->log_lock->write_lock(this->log_lock);
+       this->loggers->insert_last(this->loggers, logger);
+       this->log_lock->unlock(this->log_lock);
+}
 
-/**
- * data to remove a listener using thread_cleanup_t handler
- */
-struct cleanup_data_t {
-       /** bus instance */
-       private_bus_t *this;
-       /** listener entry */
-       entry_t *entry;
-};
+METHOD(bus_t, remove_logger, void,
+       private_bus_t *this, logger_t *logger)
+{
+       this->log_lock->write_lock(this->log_lock);
+       this->loggers->remove(this->loggers, logger, NULL);
+       this->log_lock->unlock(this->log_lock);
+}
 
 METHOD(bus_t, set_sa, void,
        private_bus_t *this, ike_sa_t *ike_sa)
@@ -198,29 +161,16 @@ typedef struct {
 } log_data_t;
 
 /**
- * listener->log() invocation as a list remove callback
+ * logger->log() invocation as a invoke_function callback
  */
-static bool log_cb(entry_t *entry, log_data_t *data)
+static void log_cb(logger_t *logger, log_data_t *data)
 {
        va_list args;
 
-       if (entry->logging)
-       {       /* avoid recursive calls */
-               return FALSE;
-       }
-       entry->logging = TRUE;
-
        va_copy(args, data->args);
-       if (!entry->listener->log(entry->listener, data->group, data->level,
-                                                         data->thread, data->ike_sa, data->format, args))
-       {
-               entry->enabled = FALSE;
-               va_end(args);
-               return TRUE;
-       }
+       logger->log(logger, data->group, data->level, data->thread, data->ike_sa,
+                               data->format, args);
        va_end(args);
-       entry->logging = FALSE;
-       return FALSE;
 }
 
 METHOD(bus_t, vlog, void,
@@ -236,11 +186,9 @@ METHOD(bus_t, vlog, void,
        data.format = format;
        va_copy(data.args, args);
 
-       this->log_mutex->lock(this->log_mutex);
-       /* We use the remove() method to invoke all listeners. This is cheap and
-        * does not require an allocation for this performance critical function. */
-       this->loggers->remove(this->loggers, &data, (void*)log_cb);
-       this->log_mutex->unlock(this->log_mutex);
+       this->log_lock->read_lock(this->log_lock);
+       this->loggers->invoke_function(this->loggers, (void*)log_cb, &data);
+       this->log_lock->unlock(this->log_lock);
 
        va_end(data.args);
 }
@@ -258,14 +206,11 @@ METHOD(bus_t, log_, void,
 /**
  * unregister a listener
  */
-static void unregister_listener(private_bus_t *this, entry_t *entry,
-                                                               enumerator_t *enumerator)
+static inline void unregister_listener(private_bus_t *this, entry_t *entry,
+                                                                          enumerator_t *enumerator)
 {
        this->listeners->remove_at(this->listeners, enumerator);
-       this->log_mutex->lock(this->log_mutex);
-       this->loggers->remove(this->loggers, entry, NULL);
-       this->log_mutex->unlock(this->log_mutex);
-       entry_destroy(entry);
+       free(entry);
 }
 
 METHOD(bus_t, alert, void,
@@ -287,15 +232,12 @@ METHOD(bus_t, alert, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       va_start(args, alert);
-                       keep = entry->listener->alert(entry->listener, ike_sa, alert, args);
-                       va_end(args);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               va_start(args, alert);
+               keep = entry->listener->alert(entry->listener, ike_sa, alert, args);
+               va_end(args);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -319,13 +261,10 @@ METHOD(bus_t, ike_state_change, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->ike_state_change(entry->listener, ike_sa, state);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->ike_state_change(entry->listener, ike_sa, state);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -352,14 +291,11 @@ METHOD(bus_t, child_state_change, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->child_state_change(entry->listener, ike_sa,
-                                                                                                          child_sa, state);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->child_state_change(entry->listener, ike_sa,
+                                                                                                  child_sa, state);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -386,14 +322,11 @@ METHOD(bus_t, message, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->message(entry->listener, ike_sa,
-                                                                                       message, incoming, plain);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->message(entry->listener, ike_sa,
+                                                                               message, incoming, plain);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -419,14 +352,11 @@ METHOD(bus_t, ike_keys, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->ike_keys(entry->listener, ike_sa, dh, dh_other,
-                                                                                        nonce_i, nonce_r, rekey, shared);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->ike_keys(entry->listener, ike_sa, dh, dh_other,
+                                                                                nonce_i, nonce_r, rekey, shared);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -454,14 +384,11 @@ METHOD(bus_t, child_keys, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->child_keys(entry->listener, ike_sa,
-                                                                       child_sa, initiator, dh, nonce_i, nonce_r);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->child_keys(entry->listener, ike_sa,
+                                                               child_sa, initiator, dh, nonce_i, nonce_r);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -488,14 +415,11 @@ METHOD(bus_t, child_updown, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->child_updown(entry->listener,
-                                                                                                ike_sa, child_sa, up);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->child_updown(entry->listener,
+                                                                                        ike_sa, child_sa, up);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -522,14 +446,11 @@ METHOD(bus_t, child_rekey, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->child_rekey(entry->listener, ike_sa,
-                                                                                               old, new);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->child_rekey(entry->listener, ike_sa,
+                                                                                       old, new);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -553,13 +474,10 @@ METHOD(bus_t, ike_updown, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->ike_updown(entry->listener, ike_sa, up);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->ike_updown(entry->listener, ike_sa, up);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -597,13 +515,10 @@ METHOD(bus_t, ike_rekey, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->ike_rekey(entry->listener, old, new);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->ike_rekey(entry->listener, old, new);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -630,14 +545,11 @@ METHOD(bus_t, authorize, bool,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->authorize(entry->listener, ike_sa,
-                                                                                         final, &success);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->authorize(entry->listener, ike_sa,
+                                                                                 final, &success);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -670,14 +582,11 @@ METHOD(bus_t, narrow, void,
                {
                        continue;
                }
-               if (entry->enabled)
-               {
-                       entry->calling++;
-                       keep = entry->listener->narrow(entry->listener, ike_sa, child_sa,
-                                                                                  type, local, remote);
-                       entry->calling--;
-               }
-               if (!entry->enabled || !keep)
+               entry->calling++;
+               keep = entry->listener->narrow(entry->listener, ike_sa, child_sa,
+                                                                          type, local, remote);
+               entry->calling--;
+               if (!keep)
                {
                        unregister_listener(this, entry, enumerator);
                }
@@ -689,11 +598,11 @@ METHOD(bus_t, narrow, void,
 METHOD(bus_t, destroy, void,
        private_bus_t *this)
 {
+       this->listeners->destroy_function(this->listeners, (void*)free);
+       this->loggers->destroy(this->loggers);
        this->thread_sa->destroy(this->thread_sa);
-       this->log_mutex->destroy(this->log_mutex);
+       this->log_lock->destroy(this->log_lock);
        this->mutex->destroy(this->mutex);
-       this->listeners->destroy_function(this->listeners, (void*)entry_destroy);
-       this->loggers->destroy(this->loggers);
        free(this);
 }
 
@@ -708,6 +617,8 @@ bus_t *bus_create()
                .public = {
                        .add_listener = _add_listener,
                        .remove_listener = _remove_listener,
+                       .add_logger = _add_logger,
+                       .remove_logger = _remove_logger,
                        .set_sa = _set_sa,
                        .get_sa = _get_sa,
                        .log = _log_,
@@ -729,7 +640,7 @@ bus_t *bus_create()
                .listeners = linked_list_create(),
                .loggers = linked_list_create(),
                .mutex = mutex_create(MUTEX_TYPE_RECURSIVE),
-               .log_mutex = mutex_create(MUTEX_TYPE_RECURSIVE),
+               .log_lock = rwlock_create(RWLOCK_TYPE_DEFAULT),
                .thread_sa = thread_value_create(NULL),
        );
 
index d5eb73a..d6e4a67 100644 (file)
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2012 Tobias Brunner
  * Copyright (C) 2006-2009 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
@@ -31,6 +32,7 @@ typedef struct bus_t bus_t;
 #include <sa/ike_sa.h>
 #include <sa/child_sa.h>
 #include <processing/jobs/job.h>
+#include <bus/listeners/logger.h>
 #include <bus/listeners/listener.h>
 
 /* undefine the definitions from libstrongswan */
@@ -118,12 +120,7 @@ enum narrow_hook_t {
 /**
  * The bus receives events and sends them to all registered listeners.
  *
- * Calls to bus_t.log() are handled seperately from calls to other event
- * functions.  This means that listeners have to be aware that calls to
- * listener_t.log() can happen concurrently with calls to one of the other
- * callbacks.  Due to this unregistering from the log() callback is not fully
- * in sync with the other callbacks, thus, one of these might be called before
- * the listener is finally unregistered.
+ * Loggers are handled separately.
  */
 struct bus_t {
 
@@ -146,6 +143,25 @@ struct bus_t {
        void (*remove_listener) (bus_t *this, listener_t *listener);
 
        /**
+        * Register a logger with the bus.
+        *
+        * The logger is passive; the thread which emitted the event
+        * processes the logger routine.  This routine may be called concurrently
+        * by multiple threads.  Recursive calls are not prevented, so logger that
+        * may cause recursive calls are responsible to avoid infinite loops.
+        *
+        * @param logger        logger to register.
+        */
+       void (*add_logger) (bus_t *this, logger_t *logger);
+
+       /**
+        * Unregister a logger from the bus.
+        *
+        * @param logger        logger to unregister.
+        */
+       void (*remove_logger) (bus_t *this, logger_t *logger);
+
+       /**
         * Set the IKE_SA the calling thread is using.
         *
         * To associate a received log message with an IKE_SA without passing it as
index 36d1861..2927340 100644 (file)
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2012 Tobias Brunner
  * Copyright (C) 2006 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
@@ -53,9 +54,9 @@ struct private_file_logger_t {
        bool ike_name;
 };
 
-METHOD(listener_t, log_, bool,
-          private_file_logger_t *this, debug_t group, level_t level, int thread,
-          ike_sa_t* ike_sa, char *format, va_list args)
+METHOD(logger_t, log_, void,
+       private_file_logger_t *this, debug_t group, level_t level, int thread,
+       ike_sa_t* ike_sa, char *format, va_list args)
 {
        if (level <= this->levels[group])
        {
@@ -112,12 +113,10 @@ METHOD(listener_t, log_, bool,
                        current = next;
                }
        }
-       /* always stay registered */
-       return TRUE;
 }
 
 METHOD(file_logger_t, set_level, void,
-          private_file_logger_t *this, debug_t group, level_t level)
+       private_file_logger_t *this, debug_t group, level_t level)
 {
        if (group < DBG_ANY)
        {
@@ -133,7 +132,7 @@ METHOD(file_logger_t, set_level, void,
 }
 
 METHOD(file_logger_t, destroy, void,
-          private_file_logger_t *this)
+       private_file_logger_t *this)
 {
        if (this->out != stdout && this->out != stderr)
        {
@@ -151,7 +150,7 @@ file_logger_t *file_logger_create(FILE *out, char *time_format, bool ike_name)
 
        INIT(this,
                .public = {
-                       .listener = {
+                       .logger = {
                                .log = _log_,
                        },
                        .set_level = _set_level,
index d02f170..85a2690 100644 (file)
@@ -21,7 +21,7 @@
 #ifndef FILE_LOGGER_H_
 #define FILE_LOGGER_H_
 
-#include <bus/listeners/listener.h>
+#include <bus/listeners/logger.h>
 
 typedef struct file_logger_t file_logger_t;
 
@@ -31,9 +31,9 @@ typedef struct file_logger_t file_logger_t;
 struct file_logger_t {
 
        /**
-        * Implements the listener_t interface.
+        * Implements the logger_t interface.
         */
-       listener_t listener;
+       logger_t logger;
 
        /**
         * Set the loglevel for a debug group.
index a567305..703e852 100644 (file)
@@ -31,33 +31,6 @@ typedef struct listener_t listener_t;
 struct listener_t {
 
        /**
-        * Log a debugging message.
-        *
-        * The implementing signal function returns TRUE to stay registered
-        * to the bus, or FALSE to unregister itself.
-        *
-        * Calling bus_t.log() inside of a registered listener is possible
-        * from all listener_t callbacks, but recursive calls from log() itself
-        * are ignored.
-        *
-        * Note that calls to bus_t.log() are handled seperately from calls to
-        * other functions, thus this callback may be called concurrently with
-        * some of the others. Because of this unregistering from this callback
-        * does not happen in sync with the other callbacks, thus, one of the other
-        * callbacks might be called before the listener is finally unregistered.
-        *
-        * @param group         kind of the signal (up, down, rekeyed, ...)
-        * @param level         verbosity level of the signal
-        * @param thread        ID of the thread raised this signal
-        * @param ike_sa        IKE_SA associated to the event
-        * @param format        printf() style format string
-        * @param args          vprintf() style va_list argument list
-        * @return                      TRUE to stay registered, FALSE to unregister
-        */
-       bool (*log)(listener_t *this, debug_t group, level_t level, int thread,
-                               ike_sa_t *ike_sa, char* format, va_list args);
-
-       /**
         * Hook called if a critical alert is risen.
         *
         * @param ike_sa        IKE_SA associated to the alert, if any
diff --git a/src/libcharon/bus/listeners/logger.h b/src/libcharon/bus/listeners/logger.h
new file mode 100644 (file)
index 0000000..9918e29
--- /dev/null
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2012 Tobias Brunner
+ * Hochschule fuer Technik Rapperswil
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.  See <http://www.fsf.org/copyleft/gpl.txt>.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * for more details.
+ */
+
+/**
+ * @defgroup logger logger
+ * @{ @ingroup listeners
+ */
+
+#ifndef LOGGER_H_
+#define LOGGER_H_
+
+typedef struct logger_t logger_t;
+
+#include <bus/bus.h>
+
+/**
+ * Logger interface, listens for log events on the bus.
+ */
+struct logger_t {
+
+       /**
+        * Log a debugging message.
+        *
+        * @note Calls to bus_t.log() are handled seperately from calls to
+        * other functions. This callback may be called concurrently by
+        * multiple threads.  Also recurisve calls are not prevented, logger that
+        * may cause recursive calls are responsible to avoid infinite loops.
+        *
+        * @param group         kind of the signal (up, down, rekeyed, ...)
+        * @param level         verbosity level of the signal
+        * @param thread        ID of the thread raised this signal
+        * @param ike_sa        IKE_SA associated to the event
+        * @param format        printf() style format string
+        * @param args          vprintf() style argument list
+        */
+       void (*log)(logger_t *this, debug_t group, level_t level, int thread,
+                               ike_sa_t *ike_sa, char* format, va_list args);
+
+};
+
+#endif /** LOGGER_H_ @}*/
index c29c9f2..f47c9ae 100644 (file)
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2012 Tobias Brunner
  * Copyright (C) 2006 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
@@ -48,9 +49,9 @@ struct private_sys_logger_t {
        bool ike_name;
 };
 
-METHOD(listener_t, log_, bool,
-          private_sys_logger_t *this, debug_t group, level_t level, int thread,
-          ike_sa_t* ike_sa, char *format, va_list args)
+METHOD(logger_t, log_, void,
+       private_sys_logger_t *this, debug_t group, level_t level, int thread,
+       ike_sa_t* ike_sa, char *format, va_list args)
 {
        if (level <= this->levels[group])
        {
@@ -59,7 +60,7 @@ METHOD(listener_t, log_, bool,
 
                /* write in memory buffer first */
                vsnprintf(buffer, sizeof(buffer), format, args);
-               /* cache group name */
+               /* cache group name and optional name string */
                snprintf(groupstr, sizeof(groupstr), "%N", debug_names, group);
 
                if (this->ike_name && ike_sa)
@@ -76,7 +77,7 @@ METHOD(listener_t, log_, bool,
                        }
                }
 
-               /* do a syslog with every line */
+               /* do a syslog for every line */
                while (current)
                {
                        next = strchr(current, '\n');
@@ -89,12 +90,10 @@ METHOD(listener_t, log_, bool,
                        current = next;
                }
        }
-       /* always stay registered */
-       return TRUE;
 }
 
 METHOD(sys_logger_t, set_level, void,
-          private_sys_logger_t *this, debug_t group, level_t level)
+       private_sys_logger_t *this, debug_t group, level_t level)
 {
        if (group < DBG_ANY)
        {
@@ -110,7 +109,7 @@ METHOD(sys_logger_t, set_level, void,
 }
 
 METHOD(sys_logger_t, destroy, void,
-          private_sys_logger_t *this)
+       private_sys_logger_t *this)
 {
        closelog();
        free(this);
@@ -125,7 +124,7 @@ sys_logger_t *sys_logger_create(int facility, bool ike_name)
 
        INIT(this,
                .public = {
-                       .listener = {
+                       .logger = {
                                .log = _log_,
                        },
                        .set_level = _set_level,
index d83715a..fcb6655 100644 (file)
@@ -21,7 +21,7 @@
 #ifndef SYS_LOGGER_H_
 #define SYS_LOGGER_H_
 
-#include <bus/listeners/listener.h>
+#include <bus/listeners/logger.h>
 
 typedef struct sys_logger_t sys_logger_t;
 
@@ -31,9 +31,9 @@ typedef struct sys_logger_t sys_logger_t;
 struct sys_logger_t {
 
        /**
-        * Implements the listener_t interface.
+        * Implements the logger_t interface.
         */
-       listener_t listener;
+       logger_t logger;
 
        /**
         * Set the loglevel for a debug group.
index 84adce0..7ba4747 100644 (file)
@@ -29,6 +29,7 @@
 
 typedef struct private_controller_t private_controller_t;
 typedef struct interface_listener_t interface_listener_t;
+typedef struct interface_logger_t interface_logger_t;
 
 /**
  * Private data of an stroke_t object.
@@ -42,19 +43,18 @@ struct private_controller_t {
 };
 
 /**
- * helper struct to map listener callbacks to interface callbacks
+ * helper struct for the logger interface
  */
-struct interface_listener_t {
-
+struct interface_logger_t {
        /**
-        * public bus listener interface
+        * public logger interface
         */
-       listener_t public;
+       logger_t public;
 
        /**
-        * status of the operation, return to method callers
+        * reference to the listener
         */
-       status_t status;
+       interface_listener_t *listener;
 
        /**
         *  interface callback (listener gets redirected to here)
@@ -65,6 +65,27 @@ struct interface_listener_t {
         * user parameter to pass to callback
         */
        void *param;
+};
+
+/**
+ * helper struct to map listener callbacks to interface callbacks
+ */
+struct interface_listener_t {
+
+       /**
+        * public bus listener interface
+        */
+       listener_t public;
+
+       /**
+        * logger interface
+        */
+       interface_logger_t logger;
+
+       /**
+        * status of the operation, return to method callers
+        */
+       status_t status;
 
        /**
         * child configuration, used for initiate
@@ -135,6 +156,7 @@ static inline bool listener_done(interface_listener_t *listener)
 static void listener_cleanup(interface_listener_t *listener)
 {
        charon->bus->remove_listener(charon->bus, &listener->public);
+       charon->bus->remove_logger(charon->bus, &listener->logger.public);
        listener->done->destroy(listener->done);
 }
 
@@ -156,6 +178,7 @@ static bool wait_for_listener(interface_listener_t *listener, job_t *job,
 
        listener->done = semaphore_create(0);
 
+       charon->bus->add_logger(charon->bus, &listener->logger.public);
        charon->bus->add_listener(charon->bus, &listener->public);
        lib->processor->queue_job(lib->processor, job);
 
@@ -174,19 +197,18 @@ static bool wait_for_listener(interface_listener_t *listener, job_t *job,
        return timed_out;
 }
 
-METHOD(listener_t, listener_log, bool,
-       interface_listener_t *this, debug_t group, level_t level, int thread,
+METHOD(logger_t, listener_log, void,
+       interface_logger_t *this, debug_t group, level_t level, int thread,
        ike_sa_t *ike_sa, char* format, va_list args)
 {
-       if (this->ike_sa == ike_sa)
+       if (this->listener->ike_sa == ike_sa)
        {
                if (!this->callback(this->param, group, level, ike_sa, format, args))
                {
-                       this->status = NEED_MORE;
-                       return listener_done(this);
+                       this->listener->status = NEED_MORE;
+                       listener_done(this->listener);
                }
        }
-       return TRUE;
 }
 
 METHOD(job_t, get_priority_medium, job_priority_t,
@@ -322,12 +344,16 @@ METHOD(controller_t, initiate, status_t,
        interface_job_t job = {
                .listener = {
                        .public = {
-                               .log = _listener_log,
                                .ike_state_change = _ike_state_change,
                                .child_state_change = _child_state_change,
                        },
-                       .callback = callback,
-                       .param = param,
+                       .logger = {
+                               .public = {
+                                       .log = _listener_log,
+                               },
+                               .callback = callback,
+                               .param = param,
+                       },
                        .status = FAILED,
                        .child_cfg = child_cfg,
                        .peer_cfg = peer_cfg,
@@ -338,6 +364,8 @@ METHOD(controller_t, initiate, status_t,
                        .destroy = _recheckin,
                },
        };
+       job.listener.logger.listener = &job.listener;
+
        if (callback == NULL)
        {
                initiate_execute(&job);
@@ -382,12 +410,16 @@ METHOD(controller_t, terminate_ike, status_t,
        interface_job_t job = {
                .listener = {
                        .public = {
-                               .log = _listener_log,
                                .ike_state_change = _ike_state_change,
                                .child_state_change = _child_state_change,
                        },
-                       .callback = callback,
-                       .param = param,
+                       .logger = {
+                               .public = {
+                                       .log = _listener_log,
+                               },
+                               .callback = callback,
+                               .param = param,
+                       },
                        .status = FAILED,
                        .id = unique_id,
                },
@@ -397,6 +429,7 @@ METHOD(controller_t, terminate_ike, status_t,
                        .destroy = _recheckin,
                },
        };
+       job.listener.logger.listener = &job.listener;
 
        ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager,
                                                                                                        unique_id, FALSE);
@@ -455,12 +488,16 @@ METHOD(controller_t, terminate_child, status_t,
        interface_job_t job = {
                .listener = {
                        .public = {
-                               .log = _listener_log,
                                .ike_state_change = _ike_state_change,
                                .child_state_change = _child_state_change,
                        },
-                       .callback = callback,
-                       .param = param,
+                       .logger = {
+                               .public = {
+                                       .log = _listener_log,
+                               },
+                               .callback = callback,
+                               .param = param,
+                       },
                        .status = FAILED,
                        .id = reqid,
                },
@@ -470,6 +507,7 @@ METHOD(controller_t, terminate_child, status_t,
                        .destroy = _recheckin,
                },
        };
+       job.listener.logger.listener = &job.listener;
 
        ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager,
                                                                                                        reqid, TRUE);
index f7624b2..fbc2a93 100644 (file)
@@ -41,7 +41,7 @@ struct private_android_logger_t {
 };
 
 
-METHOD(listener_t, log_, bool,
+METHOD(logger_t, log_, void,
           private_android_logger_t *this, debug_t group, level_t level,
           int thread, ike_sa_t* ike_sa, char *format, va_list args)
 {
@@ -64,8 +64,6 @@ METHOD(listener_t, log_, bool,
                        current = next;
                }
        }
-       /* always stay registered */
-       return TRUE;
 }
 
 METHOD(android_logger_t, destroy, void,
@@ -83,7 +81,7 @@ android_logger_t *android_logger_create()
 
        INIT(this,
                .public = {
-                       .listener = {
+                       .logger = {
                                .log = _log_,
                        },
                        .destroy = _destroy,
index c6fe5af..15abbb4 100644 (file)
@@ -31,9 +31,9 @@ typedef struct android_logger_t android_logger_t;
 struct android_logger_t {
 
        /**
-        * Implements bus_listener_t interface
+        * Implements logger_t interface
         */
-       listener_t listener;
+       logger_t logger;
 
        /**
         * Destroy the logger.
index 091f34a..bad8bc0 100644 (file)
@@ -68,7 +68,7 @@ METHOD(plugin_t, destroy, void,
        hydra->attributes->remove_handler(hydra->attributes,
                                                                          &this->handler->handler);
        lib->credmgr->remove_set(lib->credmgr, &this->creds->set);
-       charon->bus->remove_listener(charon->bus, &this->logger->listener);
+       charon->bus->remove_logger(charon->bus, &this->logger->logger);
        this->creds->destroy(this->creds);
        this->handler->destroy(this->handler);
        this->logger->destroy(this->logger);
@@ -98,7 +98,7 @@ plugin_t *android_plugin_create()
        this->service = android_service_create(this->creds);
        this->handler = android_handler_create(this->service != NULL);
 
-       charon->bus->add_listener(charon->bus, &this->logger->listener);
+       charon->bus->add_logger(charon->bus, &this->logger->logger);
        lib->credmgr->add_set(lib->credmgr, &this->creds->set);
        hydra->attributes->add_handler(hydra->attributes, &this->handler->handler);
 
index 10ceacb..e693bac 100644 (file)
@@ -18,6 +18,7 @@
 #include "sql_logger.h"
 
 #include <daemon.h>
+#include <threading/thread_value.h>
 
 typedef struct private_sql_logger_t private_sql_logger_t;
 
@@ -42,20 +43,20 @@ struct private_sql_logger_t {
        int level;
 
        /**
-        * avoid recursive logging
+        * avoid recursive calls by the same thread
         */
-       bool recursive;
+       thread_value_t *recursive;
 };
 
-METHOD(listener_t, log_, bool,
+METHOD(logger_t, log_, void,
        private_sql_logger_t *this, debug_t group, level_t level, int thread,
        ike_sa_t* ike_sa, char *format, va_list args)
 {
-       if (this->recursive)
+       if (this->recursive->get(this->recursive))
        {
-               return TRUE;
+               return;
        }
-       this->recursive = TRUE;
+       this->recursive->set(this->recursive, this->recursive);
 
        if (ike_sa && level <= this->level)
        {
@@ -108,9 +109,7 @@ METHOD(listener_t, log_, bool,
                                                  DB_BLOB, local_spi, DB_INT, group, DB_INT, level,
                                                  DB_TEXT, buffer);
        }
-       this->recursive = FALSE;
-       /* always stay registered */
-       return TRUE;
+       this->recursive->set(this->recursive, NULL);
 }
 
 METHOD(sql_logger_t, destroy, void,
@@ -128,12 +127,13 @@ sql_logger_t *sql_logger_create(database_t *db)
 
        INIT(this,
                .public = {
-                       .listener = {
+                       .logger = {
                                .log = _log_,
                        },
                        .destroy = _destroy,
                },
                .db = db,
+               .recursive = thread_value_create(NULL),
                .level = lib->settings->get_int(lib->settings,
                                                                                "charon.plugins.sql.loglevel", -1),
        );
index a933705..62dc3f3 100644 (file)
@@ -32,9 +32,9 @@ typedef struct sql_logger_t sql_logger_t;
 struct sql_logger_t {
 
        /**
-        * Implements bus_listener_t interface
+        * Implements logger_t interface
         */
-       listener_t listener;
+       logger_t logger;
 
        /**
         * Destry the backend.
index d915d46..fc05fa5 100644 (file)
@@ -64,7 +64,7 @@ METHOD(plugin_t, destroy, void,
 {
        charon->backends->remove_backend(charon->backends, &this->config->backend);
        lib->credmgr->remove_set(lib->credmgr, &this->cred->set);
-       charon->bus->remove_listener(charon->bus, &this->logger->listener);
+       charon->bus->remove_logger(charon->bus, &this->logger->logger);
        this->config->destroy(this->config);
        this->cred->destroy(this->cred);
        this->logger->destroy(this->logger);
@@ -110,7 +110,7 @@ plugin_t *sql_plugin_create()
 
        charon->backends->add_backend(charon->backends, &this->config->backend);
        lib->credmgr->add_set(lib->credmgr, &this->cred->set);
-       charon->bus->add_listener(charon->bus, &this->logger->listener);
+       charon->bus->add_logger(charon->bus, &this->logger->logger);
 
        return &this->public.plugin;
 }