From: Sebastian Harl <sh@tokkee.org>
Date: Tue, 24 Feb 2009 10:27:22 +0000 (+0100)
Subject: src/common.c: Fixed a race condition in check_create_dir().
X-Git-Tag: collectd-4.6.2~12^2~1
X-Git-Url: https://git.verplant.org/?a=commitdiff_plain;h=da70cd0f24be40b2d76a9dd88ff8264b52c989bd;p=collectd.git

src/common.c: Fixed a race condition in check_create_dir().

Between checking for the existence of a directory using stat() and creating
the directory using mkdir(), another thread might have already created the
directory thus causing mkdir() to fail with errno == EEXIST. This case is now
handled sanely, no longer causing the function (and thus some write callback)
to fail in this case.

Most likely, this only happens during startup when creating the data
directories - later, no two threads should try to create the same directory.

Interestingly enough, I frequently encountered this issue on a single core
machine.
---

diff --git a/src/common.c b/src/common.c
index 119d284f..182f923d 100644
--- a/src/common.c
+++ b/src/common.c
@@ -433,7 +433,8 @@ int check_create_dir (const char *file_orig)
 		 */
 		if (fields[i][0] == '.')
 		{
-			ERROR ("Cowardly refusing to create a directory that begins with a `.' (dot): `%s'", file_orig);
+			ERROR ("Cowardly refusing to create a directory that "
+					"begins with a `.' (dot): `%s'", file_orig);
 			return (-2);
 		}
 
@@ -448,32 +449,42 @@ int check_create_dir (const char *file_orig)
 			return (-1);
 		}
 
-		if (stat (dir, &statbuf) == -1)
-		{
-			if (errno == ENOENT)
+		while (42) {
+			if (stat (dir, &statbuf) == -1)
 			{
-				if (mkdir (dir, 0755) == -1)
+				if (errno == ENOENT)
 				{
+					if (mkdir (dir, 0755) == 0)
+						break;
+
+					/* this might happen, if a different thread created
+					 * the directory in the meantime
+					 * => call stat() again to check for S_ISDIR() */
+					if (EEXIST == errno)
+						continue;
+
 					char errbuf[1024];
 					ERROR ("check_create_dir: mkdir (%s): %s", dir,
 							sstrerror (errno,
 								errbuf, sizeof (errbuf)));
 					return (-1);
 				}
+				else
+				{
+					char errbuf[1024];
+					ERROR ("check_create_dir: stat (%s): %s", dir,
+							sstrerror (errno, errbuf,
+								sizeof (errbuf)));
+					return (-1);
+				}
 			}
-			else
+			else if (!S_ISDIR (statbuf.st_mode))
 			{
-				char errbuf[1024];
-				ERROR ("stat (%s): %s", dir,
-						sstrerror (errno, errbuf,
-							sizeof (errbuf)));
+				ERROR ("check_create_dir: `%s' exists but is not "
+						"a directory!", dir);
 				return (-1);
 			}
-		}
-		else if (!S_ISDIR (statbuf.st_mode))
-		{
-			ERROR ("stat (%s): Not a directory!", dir);
-			return (-1);
+			break;
 		}
 	}