Revert: Semmle: Protect against theoretical race in recursive d… (#3619)

As discussed with @JelteF; #3559 caused consistent errors on BSD (OSX). Given a group of people use this environment to develop on it is an undesirable change.

This reverts commit ca8f7119fe.

src/backend/distributed/worker/worker_partition_protocol.c

@@ -85,8 +85,7 @@ static uint32 RangePartitionId(Datum partitionValue, Oid partitionCollation,
 static uint32 HashPartitionId(Datum partitionValue, Oid partitionCollation,
 							  const void *context);
 static StringInfo UserPartitionFilename(StringInfo directoryName, uint32 partitionId);
-static bool TryUnlink(const char *filename);
+static bool FileIsLink(const char *filename, struct stat filestat);
-static bool TryRmdir(const char *filename);
 
 
 /* exports for SQL callable functions */
@@ -689,6 +688,25 @@ CitusCreateDirectory(StringInfo directoryName)
 }
 
 
+#ifdef WIN32
+static bool
+FileIsLink(char *filename, struct stat filestat)
+{
+	return pgwin32_is_junction(filename);
+}
+
+
+#else
+static bool
+FileIsLink(const char *filename, struct stat filestat)
+{
+	return S_ISLNK(filestat.st_mode);
+}
+
+
+#endif
+
+
 /*
  * CitusRemoveDirectory first checks if the given directory exists. If it does, the
  * function recursively deletes the contents of the given directory, and then
@@ -701,34 +719,42 @@ CitusRemoveDirectory(const char *filename)
 	/* files may be added during execution, loop when that occurs */
 	while (true)
 	{
-		if (TryUnlink(filename) || TryRmdir(filename))
+		struct stat fileStat;
+		int removed = 0;
+
+		int statOK = stat(filename, &fileStat);
+		if (statOK < 0)
 		{
-			return;
+			if (errno == ENOENT)
+			{
+				return;  /* if file does not exist, return */
+			}
+			else
+			{
+				ereport(ERROR, (errcode_for_file_access(),
+								errmsg("could not stat file \"%s\": %m", filename)));
+			}
 		}
 
 		/*
-		 * If both of these failed then filename is a non empty directory.
+		 * If this is a directory, iterate over all its contents and for each
-		 * Iterate over all its contents and for each content, recurse into
+		 * content, recurse into this function. Also, make sure that we do not
-		 * this function. Also, make sure that we do not recurse into symbolic
+		 * recurse into symbolic links.
-		 * links.
 		 */
+		if (S_ISDIR(fileStat.st_mode) && !FileIsLink(filename, fileStat))
+		{
 			const char *directoryName = filename;
 
 			DIR *directory = AllocateDir(directoryName);
-		if (errno == ENOENT)
+			if (directory == NULL)
 			{
-			/* If directory was removed from under us we're done */
+				ereport(ERROR, (errcode_for_file_access(),
-			return;
+								errmsg("could not open directory \"%s\": %m",
-		}
+									   directoryName)));
-		if (errno == ENOTDIR)
-		{
-			/* If directory changed to a file underneath us, loop again to remove it with unlink */
-			continue;
 			}
 
-		/* ReadDir handles other errno from AllocateDir */
-		struct dirent *directoryEntry = ReadDir(directory, directoryName);
 			StringInfo fullFilename = makeStringInfo();
+			struct dirent *directoryEntry = ReadDir(directory, directoryName);
 			for (; directoryEntry != NULL; directoryEntry = ReadDir(directory,
 																	directoryName))
 			{
@@ -750,53 +776,30 @@ CitusRemoveDirectory(const char *filename)
 			FreeStringInfo(fullFilename);
 			FreeDir(directory);
 		}
+
+		/* we now have an empty directory or a regular file, remove it */
+		if (S_ISDIR(fileStat.st_mode))
+		{
+			removed = rmdir(filename);
+
+			if (errno == ENOTEMPTY || errno == EEXIST)
+			{
+				continue;
+			}
+		}
+		else
+		{
+			removed = unlink(filename);
 		}
 
-
+		if (removed != 0 && errno != ENOENT)
-/*
- * TryUnlink returns true if file is removed and false if the file is a
- * directory. If an error occurs trying to remove the file this function calls
- * ereport.
- */
-static bool
-TryUnlink(const char *filename)
 		{
-	errno = 0;
-	int exitcode = unlink(filename);
-	if (exitcode == 0 || errno == ENOENT)
-	{
-		return true;
-	}
-	if (errno == EISDIR)
-	{
-		return false;
-	}
 			ereport(ERROR, (errcode_for_file_access(),
 							errmsg("could not remove file \"%s\": %m", filename)));
 		}
 
-
+		return;
-/*
- * TryRmdir returns true if the directory is removed. It returns false if the
- * filename points to a file or symlink. It also returns false if the directory
- * is not empty. The difference in these cases can be found by checking errno.
- * For all other errors ereport is called.
- */
-static bool
-TryRmdir(const char *filename)
-{
-	errno = 0;
-	int exitcode = rmdir(filename);
-	if (exitcode == 0 || errno == ENOENT)
-	{
-		return true;
 	}
-	if (errno == ENOTDIR || errno == ENOTEMPTY || errno == EEXIST)
-	{
-		return false;
-	}
-	ereport(ERROR, (errcode_for_file_access(),
-					errmsg("could not remove directory \"%s\": %m", filename)));
 }