Create a job object during Windows tests to kill abandoned daemons

Should prevent some cases of tests hanging on AppVeyor due to child
process daemons that were started and not stopped.

Delete files between tests using much more similar code (and the same
list) on Windows and Linux, to reduce risk of divergence causing tests
to pass on one and fail on the other.

infrastructure/buildenv-testmain-template.cpp

@@ -371,6 +371,37 @@ int main(int argc, char * const * argv)
 		memleakfinder_init();
 		#endif
 
+#ifdef WIN32
+		// Create a Windows "Job Object" for Test.cpp to use as a
+		// container for all our child processes (daemons). We will
+		// close the handle (killing any running daemons) when we exit.
+		// This is the best way to avoid daemons hanging around and
+		// causing subsequent tests to fail, and/or the test runner to
+		// hang waiting for a daemon that will never terminate.
+
+		sTestChildDaemonJobObject = CreateJobObject(NULL, NULL);
+		if(sTestChildDaemonJobObject == INVALID_HANDLE_VALUE)
+		{
+			BOX_LOG_WIN_WARNING("Failed to create job object "
+				"to contain child daemons");
+		}
+		else
+		{
+			JOBOBJECT_EXTENDED_LIMIT_INFORMATION limits;
+			limits.BasicLimitInformation.LimitFlags =
+				JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
+			if(!SetInformationJobObject(
+				sTestChildDaemonJobObject,
+				JobObjectExtendedLimitInformation,
+				&limits,
+				sizeof(limits)))
+			{
+				BOX_LOG_WIN_WARNING("Failed to set limits on "
+					"job object for child daemons");
+			}
+		}
+#endif // WIN32
+
 		Timers::Init();
 		int returncode = test(argc, (const char **)argv);
 		Timers::Cleanup(false);
@@ -436,5 +467,9 @@ int main(int argc, char * const * argv)
 			printf("WARNING: Files were left open\n");
 		}
 	}
+
+#ifdef WIN32
+	CloseHandle(sTestChildDaemonJobObject);
+#endif
 }
 

lib/common/Test.cpp

@@ -17,8 +17,12 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#ifdef HAVE_DIRENT_H
+#	include <dirent.h>
+#endif
+
 #ifdef HAVE_UNISTD_H
-	#include <unistd.h>
+#	include <unistd.h>
 #endif
 
 #include "BoxTime.h"
@@ -83,7 +87,6 @@ bool setUp(const char* function_name)
 		}
 	}
 
-#ifdef _MSC_VER
 	DIR* pDir = opendir("testfiles");
 	if(!pDir)
 	{
@@ -101,13 +104,16 @@ bool setUp(const char* function_name)
 			StartsWith("notifyran", filename) ||
 			StartsWith("notifyscript.tag", filename) ||
 			StartsWith("restore", filename) ||
+			filename == "store" ||
+			filename == "bbackupd-cache" ||
 			filename == "bbackupd-data" ||
 			filename == "syncallowscript.control" ||
 			StartsWith("syncallowscript.notifyran.", filename) ||
 			filename == "test2.downloaded" ||
 			EndsWith("testfile", filename))
 		{
-			std::string filepath = std::string("testfiles\\") + filename;
+			std::string filepath = std::string("testfiles" 
+				DIRECTORY_SEPARATOR) + filename;
 
 			int filetype = ObjectExists(filepath);
 			if(filetype == ObjectExists_File)
@@ -120,6 +126,7 @@ bool setUp(const char* function_name)
 			}
 			else if(filetype == ObjectExists_Dir)
 			{
+#ifdef WIN32
 				std::string cmd = "cmd /c rd /s /q " + filepath;
 				WCHAR* wide_cmd = ConvertUtf8ToWideString(cmd.c_str());
 				if(wide_cmd == NULL)
@@ -181,6 +188,11 @@ bool setUp(const char* function_name)
 
 				CloseHandle(pi.hProcess);
 				CloseHandle(pi.hThread);
+#else // !WIN32
+				// Deleting directories is so much easier on Unix!
+				std::string cmd = "rm -rf '" + filepath + "'";
+				TEST_THAT_THROWONFAIL(system(cmd.c_str()) == 0);
+#endif // WIN32
 			}
 			else
 			{
@@ -190,21 +202,10 @@ bool setUp(const char* function_name)
 		}
 	}
 	closedir(pDir);
+
 	FileStream touch("testfiles/accounts.txt", O_WRONLY | O_CREAT | O_TRUNC,
 		S_IRUSR | S_IWUSR);
-#else
-	TEST_THAT_THROWONFAIL(system(
-		"rm -rf testfiles/TestDir* testfiles/0_0 testfiles/0_1 "
-		"testfiles/0_2 testfiles/accounts.txt " // testfiles/test* .tgz!
-		"testfiles/file* testfiles/notifyran testfiles/notifyran.* "
-		"testfiles/notifyscript.tag* "
-		"testfiles/restore* testfiles/bbackupd-data "
-		"testfiles/syncallowscript.control "
-		"testfiles/syncallowscript.notifyran.* "
-		"testfiles/test2.downloaded"
-		) == 0);
-	TEST_THAT_THROWONFAIL(system("touch testfiles/accounts.txt") == 0);
-#endif
+
 	TEST_THAT_THROWONFAIL(mkdir("testfiles/0_0", 0755) == 0);
 	TEST_THAT_THROWONFAIL(mkdir("testfiles/0_1", 0755) == 0);
 	TEST_THAT_THROWONFAIL(mkdir("testfiles/0_2", 0755) == 0);
@@ -382,12 +383,23 @@ int ReadPidFile(const char *pidFile)
 	return pid;
 }
 
+#ifdef WIN32
+HANDLE sTestChildDaemonJobObject = INVALID_HANDLE_VALUE;
+#endif
+
 int LaunchServer(const std::string& rCommandLine, const char *pidFile)
 {
 	BOX_INFO("Starting server: " << rCommandLine);
 
 #ifdef WIN32
 
+	// Use a Windows "Job Object" as a container for all our child
+	// processes. The test runner will create this job object when
+	// it starts, and close the handle (killing any running daemons)
+	// when it exits. This is the best way to avoid daemons hanging
+	// around and causing subsequent tests to fail, and/or the test
+	// runner to hang waiting for a daemon that will never terminate.
+
 	PROCESS_INFORMATION procInfo;
 
 	STARTUPINFO startInfo;
@@ -419,10 +431,18 @@ int LaunchServer(const std::string& rCommandLine, const char *pidFile)
 	free(tempCmd);
 
 	TEST_THAT_OR(result != 0,
-		BOX_LOG_WIN_ERROR("Launch failed: " << rCommandLine);
+		BOX_LOG_WIN_ERROR("Failed to CreateProcess: " << rCommandLine);
 		return -1;
 		);
 
+	if(sTestChildDaemonJobObject != INVALID_HANDLE_VALUE)
+	{
+		if(!AssignProcessToJobObject(sTestChildDaemonJobObject, procInfo.hProcess))
+		{
+			BOX_LOG_WIN_WARNING("Failed to add child process to job object");
+		}
+	}
+
 	CloseHandle(procInfo.hProcess);
 	CloseHandle(procInfo.hThread);
 

lib/common/Test.h

@@ -42,6 +42,10 @@ extern std::list<std::string> run_only_named_tests;
 extern std::string current_test_name;
 extern std::map<std::string, std::string> s_test_status;
 
+#ifdef WIN32
+extern HANDLE sTestChildDaemonJobObject;
+#endif
+
 //! Simplifies calling setUp() with the current function name in each test.
 #define SETUP() \
 	if (!setUp(__FUNCTION__)) return true; \