rrdtool plugin: fix thread-safety detection
[collectd.git] / src / utils_rrdcreate.c
index 2228660..1bd1ad4 100644 (file)
@@ -36,6 +36,14 @@ struct srrd_create_args_s
 };
 typedef struct srrd_create_args_s srrd_create_args_t;
 
+struct async_create_file_s;
+typedef struct async_create_file_s async_create_file_t;
+struct async_create_file_s
+{
+  char *filename;
+  async_create_file_t *next;
+};
+
 /*
  * Private variables
  */
@@ -61,6 +69,9 @@ static int rra_types_num = STATIC_ARRAY_SIZE (rra_types);
 static pthread_mutex_t librrd_lock = PTHREAD_MUTEX_INITIALIZER;
 #endif
 
+static async_create_file_t *async_creation_list = NULL;
+static pthread_mutex_t async_creation_lock = PTHREAD_MUTEX_INITIALIZER;
+
 /*
  * Private functions
  */
@@ -88,6 +99,7 @@ static void srrd_create_args_destroy (srrd_create_args_t *args)
       sfree (args->argv[i]);
     sfree (args->argv);
   }
+  sfree (args);
 } /* void srrd_create_args_destroy */
 
 static srrd_create_args_t *srrd_create_args_create (const char *filename,
@@ -154,14 +166,10 @@ static int rra_get (char ***ret, const value_list_t *vl, /* {{{ */
 
   int rra_max;
 
-  int span;
-
   int cdp_num;
   int cdp_len;
   int i, j;
 
-  char buffer[128];
-
   /* The stepsize we use here: If it is user-set, use it. If not, use the
    * interval of the value-list. */
   int ss;
@@ -201,16 +209,17 @@ static int rra_get (char ***ret, const value_list_t *vl, /* {{{ */
   }
 
   rra_max = rts_num * rra_types_num;
+  assert (rra_max > 0);
 
-  if ((rra_def = (char **) malloc ((rra_max + 1) * sizeof (char *))) == NULL)
+  if ((rra_def = malloc ((rra_max + 1) * sizeof (*rra_def))) == NULL)
     return (-1);
-  memset (rra_def, '\0', (rra_max + 1) * sizeof (char *));
+  memset (rra_def, 0, (rra_max + 1) * sizeof (*rra_def));
   rra_num = 0;
 
   cdp_len = 0;
   for (i = 0; i < rts_num; i++)
   {
-    span = rts[i];
+    int span = rts[i];
 
     if ((span / ss) < cfg->rrarows)
       span = ss * cfg->rrarows;
@@ -226,6 +235,7 @@ static int rra_get (char ***ret, const value_list_t *vl, /* {{{ */
 
     for (j = 0; j < rra_types_num; j++)
     {
+      char buffer[128];
       int status;
 
       if (rra_num >= rra_max)
@@ -244,6 +254,12 @@ static int rra_get (char ***ret, const value_list_t *vl, /* {{{ */
     }
   }
 
+  if (rra_num <= 0)
+  {
+    sfree (rra_def);
+    return (0);
+  }
+
   *ret = rra_def;
   return (rra_num);
 } /* }}} int rra_get */
@@ -269,7 +285,9 @@ static int ds_get (char ***ret, /* {{{ */
   char max[32];
   char buffer[128];
 
-  ds_def = (char **) malloc (ds->ds_num * sizeof (char *));
+  assert (ds->ds_num > 0);
+
+  ds_def = malloc (ds->ds_num * sizeof (*ds_def));
   if (ds_def == NULL)
   {
     char errbuf[1024];
@@ -277,7 +295,7 @@ static int ds_get (char ***ret, /* {{{ */
         sstrerror (errno, errbuf, sizeof (errbuf)));
     return (-1);
   }
-  memset (ds_def, '\0', ds->ds_num * sizeof (char *));
+  memset (ds_def, 0, ds->ds_num * sizeof (*ds_def));
 
   for (ds_num = 0; ds_num < ds->ds_num; ds_num++)
   {
@@ -335,6 +353,12 @@ static int ds_get (char ***ret, /* {{{ */
     return (-1);
   }
 
+  if (ds_num <= 0)
+  {
+    sfree (ds_def);
+    return (0);
+  }
+
   *ret = ds_def;
   return (ds_num);
 } /* }}} int ds_get */
@@ -434,30 +458,152 @@ static int srrd_create (const char *filename, /* {{{ */
 } /* }}} int srrd_create */
 #endif /* !HAVE_THREADSAFE_LIBRRD */
 
-static void *srrd_create_thread (void *targs)
+static int lock_file (char const *filename) /* {{{ */
+{
+  async_create_file_t *ptr;
+  struct stat sb;
+  int status;
+
+  pthread_mutex_lock (&async_creation_lock);
+
+  for (ptr = async_creation_list; ptr != NULL; ptr = ptr->next)
+    if (strcmp (filename, ptr->filename) == 0)
+      break;
+
+  if (ptr != NULL)
+  {
+    pthread_mutex_unlock (&async_creation_lock);
+    return (EEXIST);
+  }
+
+  status = stat (filename, &sb);
+  if ((status == 0) || (errno != ENOENT))
+  {
+    pthread_mutex_unlock (&async_creation_lock);
+    return (EEXIST);
+  }
+
+  ptr = malloc (sizeof (*ptr));
+  if (ptr == NULL)
+  {
+    pthread_mutex_unlock (&async_creation_lock);
+    return (ENOMEM);
+  }
+
+  ptr->filename = strdup (filename);
+  if (ptr->filename == NULL)
+  {
+    pthread_mutex_unlock (&async_creation_lock);
+    sfree (ptr);
+    return (ENOMEM);
+  }
+
+  ptr->next = async_creation_list;
+  async_creation_list = ptr;
+
+  pthread_mutex_unlock (&async_creation_lock);
+
+  return (0);
+} /* }}} int lock_file */
+
+static int unlock_file (char const *filename) /* {{{ */
+{
+  async_create_file_t *this;
+  async_create_file_t *prev;
+
+
+  pthread_mutex_lock (&async_creation_lock);
+
+  prev = NULL;
+  for (this = async_creation_list; this != NULL; this = this->next)
+  {
+    if (strcmp (filename, this->filename) == 0)
+      break;
+    prev = this;
+  }
+
+  if (this == NULL)
+  {
+    pthread_mutex_unlock (&async_creation_lock);
+    return (ENOENT);
+  }
+
+  if (prev == NULL)
+  {
+    assert (this == async_creation_list);
+    async_creation_list = this->next;
+  }
+  else
+  {
+    assert (this == prev->next);
+    prev->next = this->next;
+  }
+  this->next = NULL;
+
+  pthread_mutex_unlock (&async_creation_lock);
+
+  sfree (this->filename);
+  sfree (this);
+
+  return (0);
+} /* }}} int unlock_file */
+
+static void *srrd_create_thread (void *targs) /* {{{ */
 {
   srrd_create_args_t *args = targs;
+  char tmpfile[PATH_MAX];
   int status;
 
-  status = srrd_create (args->filename, args->pdp_step, args->last_up,
+  status = lock_file (args->filename);
+  if (status != 0)
+  {
+    if (status == EEXIST)
+      NOTICE ("srrd_create_thread: File \"%s\" is already being created.",
+          args->filename);
+    else
+      ERROR ("srrd_create_thread: Unable to lock file \"%s\".",
+          args->filename);
+    srrd_create_args_destroy (args);
+    return (0);
+  }
+
+  ssnprintf (tmpfile, sizeof (tmpfile), "%s.async", args->filename);
+
+  status = srrd_create (tmpfile, args->pdp_step, args->last_up,
       args->argc, (void *) args->argv);
   if (status != 0)
   {
     WARNING ("srrd_create_thread: srrd_create (%s) returned status %i.",
         args->filename, status);
+    unlink (tmpfile);
+    unlock_file (args->filename);
+    srrd_create_args_destroy (args);
+    return (0);
   }
-  else
+
+  status = rename (tmpfile, args->filename);
+  if (status != 0)
   {
-    DEBUG ("srrd_create_thread: Successfully created RRD file \"%s\".",
-        args->filename);
+    char errbuf[1024];
+    ERROR ("srrd_create_thread: rename (\"%s\", \"%s\") failed: %s",
+        tmpfile, args->filename,
+        sstrerror (errno, errbuf, sizeof (errbuf)));
+    unlink (tmpfile);
+    unlock_file (args->filename);
+    srrd_create_args_destroy (args);
+    return (0);
   }
 
+  DEBUG ("srrd_create_thread: Successfully created RRD file \"%s\".",
+      args->filename);
+
+  unlock_file (args->filename);
   srrd_create_args_destroy (args);
 
   return (0);
-} /* void *srrd_create_thread */
+} /* }}} void *srrd_create_thread */
 
-static int srrd_create_async (const char *filename,
+static int srrd_create_async (const char *filename, /* {{{ */
     unsigned long pdp_step, time_t last_up,
     int argc, const char **argv)
 {
@@ -466,17 +612,26 @@ static int srrd_create_async (const char *filename,
   pthread_attr_t attr;
   int status;
 
+  DEBUG ("srrd_create_async: Creating \"%s\" in the background.", filename);
+
   args = srrd_create_args_create (filename, pdp_step, last_up, argc, argv);
   if (args == NULL)
     return (-1);
 
   status = pthread_attr_init (&attr);
   if (status != 0)
+  {
+    srrd_create_args_destroy (args);
     return (-1);
+  }
 
   status = pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
   if (status != 0)
+  {
+    pthread_attr_destroy (&attr);
+    srrd_create_args_destroy (args);
     return (-1);
+  }
 
   status = pthread_create (&thread, &attr, srrd_create_thread, args);
   if (status != 0)
@@ -490,8 +645,9 @@ static int srrd_create_async (const char *filename,
   }
 
   pthread_attr_destroy (&attr);
+  /* args is freed in srrd_create_thread(). */
   return (0);
-}
+} /* }}} int srrd_create_async */
 
 /*
  * Public functions
@@ -502,9 +658,9 @@ int cu_rrd_create_file (const char *filename, /* {{{ */
 {
   char **argv;
   int argc;
-  char **rra_def;
+  char **rra_def = NULL;
   int rra_num;
-  char **ds_def;
+  char **ds_def = NULL;
   int ds_num;
   int status = 0;
   time_t last_up;
@@ -522,6 +678,7 @@ int cu_rrd_create_file (const char *filename, /* {{{ */
   if ((ds_num = ds_get (&ds_def, ds, vl, cfg)) < 1)
   {
     ERROR ("cu_rrd_create_file failed: Could not calculate DSes");
+    rra_free (rra_num, rra_def);
     return (-1);
   }
 
@@ -532,6 +689,8 @@ int cu_rrd_create_file (const char *filename, /* {{{ */
     char errbuf[1024];
     ERROR ("cu_rrd_create_file failed: %s",
         sstrerror (errno, errbuf, sizeof (errbuf)));
+    rra_free (rra_num, rra_def);
+    ds_free (ds_num, ds_def);
     return (-1);
   }
 
@@ -560,18 +719,32 @@ int cu_rrd_create_file (const char *filename, /* {{{ */
   }
   else /* synchronous */
   {
-    status = srrd_create (filename, stepsize, last_up,
-        argc, (const char **) argv);
-
+    status = lock_file (filename);
     if (status != 0)
     {
-      WARNING ("cu_rrd_create_file: srrd_create (%s) returned status %i.",
-          filename, status);
+      if (status == EEXIST)
+        NOTICE ("cu_rrd_create_file: File \"%s\" is already being created.",
+            filename);
+      else
+        ERROR ("cu_rrd_create_file: Unable to lock file \"%s\".",
+            filename);
     }
     else
     {
-      DEBUG ("cu_rrd_create_file: Successfully created RRD file \"%s\".",
-          filename);
+      status = srrd_create (filename, stepsize, last_up,
+          argc, (const char **) argv);
+
+      if (status != 0)
+      {
+        WARNING ("cu_rrd_create_file: srrd_create (%s) returned status %i.",
+            filename, status);
+      }
+      else
+      {
+        DEBUG ("cu_rrd_create_file: Successfully created RRD file \"%s\".",
+            filename);
+      }
+      unlock_file (filename);
     }
   }