rrdtool plugin: Fix a possible race condition at startup.

On a very busy system, if the network plugin was initialized before the
rrdtool plugin, `rrd_cache_insert' may be called before the RRDtool
plugin is initialized. It would then pass `cache == NULL' to
`c_avl_get', resulting in a segmentation fault.

diff --git a/src/rrdtool.c b/src/rrdtool.c
index e1c7902..27e64c1 100644 (file)
--- a/src/rrdtool.c
+++ b/src/rrdtool.c
@@ -621,6 +621,7 @@ static void *rrd_queue_thread (void *data)
                rrd_cache_t *cache_entry;
                char **values;
                int    values_num;
                rrd_cache_t *cache_entry;
                char **values;
                int    values_num;

+               int    status;

                int    i;
 
                 pthread_mutex_lock (&queue_lock);
                int    i;
 
                 pthread_mutex_lock (&queue_lock);


@@ -628,7 +629,6 @@ static void *rrd_queue_thread (void *data)
                 while (true)
                 {
                   struct timespec ts_wait;
                 while (true)
                 {
                   struct timespec ts_wait;

-                  int status;

 
                   while ((flushq_head == NULL) && (queue_head == NULL)
                       && (do_shutdown == 0))
 
                   while ((flushq_head == NULL) && (queue_head == NULL)
                       && (do_shutdown == 0))


@@ -704,17 +704,28 @@ static void *rrd_queue_thread (void *data)
                 * we make a copy of it's values */
                pthread_mutex_lock (&cache_lock);
 
                 * we make a copy of it's values */
                pthread_mutex_lock (&cache_lock);
 

-               c_avl_get (cache, queue_entry->filename, (void *) &cache_entry);
+               status = c_avl_get (cache, queue_entry->filename,
+                               (void *) &cache_entry);

 
 

-               values = cache_entry->values;
-               values_num = cache_entry->values_num;
+               if (status == 0)
+               {
+                       values = cache_entry->values;
+                       values_num = cache_entry->values_num;

 
 

-               cache_entry->values = NULL;
-               cache_entry->values_num = 0;
-               cache_entry->flags = FLAG_NONE;
+                       cache_entry->values = NULL;
+                       cache_entry->values_num = 0;
+                       cache_entry->flags = FLAG_NONE;
+               }

 
                pthread_mutex_unlock (&cache_lock);
 
 
                pthread_mutex_unlock (&cache_lock);
 

+               if (status != 0)
+               {
+                       sfree (queue_entry->filename);
+                       sfree (queue_entry);
+                       continue;
+               }
+

                /* Update `tv_next_update' */
                if (write_rate > 0.0) 
                 {
                /* Update `tv_next_update' */
                if (write_rate > 0.0) 
                 {


@@ -968,6 +979,15 @@ static int rrd_cache_insert (const char *filename,
 
        pthread_mutex_lock (&cache_lock);
 
 
        pthread_mutex_lock (&cache_lock);
 

+       /* This shouldn't happen, but it did happen at least once, so we'll be
+        * careful. */
+       if (cache == NULL)
+       {
+               pthread_mutex_unlock (&cache_lock);
+               WARNING ("rrdtool plugin: cache == NULL.");
+               return (-1);
+       }
+

        c_avl_get (cache, filename, (void *) &rc);
 
        if (rc == NULL)
        c_avl_get (cache, filename, (void *) &rc);
 
        if (rc == NULL)

diff --git a/src/utils_avltree.c b/src/utils_avltree.c
index 9f0b796..3e258e9 100644 (file)
--- a/src/utils_avltree.c
+++ b/src/utils_avltree.c
@@ -581,6 +581,8 @@ int c_avl_get (c_avl_tree_t *t, const void *key, void **value)
 {
        c_avl_node_t *n;
 
 {
        c_avl_node_t *n;
 

+       assert (t != NULL);
+

        n = search (t, key);
        if (n == NULL)
                return (-1);
        n = search (t, key);
        if (n == NULL)
                return (-1);