소스 검색

Fix issues when invoking DXC from multiple threads

Panagiotis Christopoulos Charitos 3 년 전
부모
커밋
fca3928e8b
4개의 변경된 파일112개의 추가작업 그리고 33개의 파일을 삭제
  1. 41 24
      AnKi/ShaderCompiler/Dxc.cpp
  2. 2 1
      AnKi/Util/Filesystem.cpp
  3. 48 7
      AnKi/Util/Process.cpp
  4. 21 1
      AnKi/Util/Process.h

+ 41 - 24
AnKi/ShaderCompiler/Dxc.cpp

@@ -7,6 +7,7 @@
 #include <AnKi/Util/Process.h>
 #include <AnKi/Util/Filesystem.h>
 #include <AnKi/Util/File.h>
+#include <AnKi/Util/HighRezTimer.h>
 
 namespace anki {
 
@@ -24,12 +25,12 @@ public:
 
 	~CleanupFile()
 	{
-		if(!m_fileToDelete.isEmpty())
+		if(!m_fileToDelete.isEmpty() && fileExists(m_fileToDelete))
 		{
-			const Error err = removeFile(m_fileToDelete);
-			if(!err)
+			// Loop until success because Windows antivirus may be keeping the file in use
+			while(std::remove(m_fileToDelete.cstr()))
 			{
-				ANKI_SHADER_COMPILER_LOGV("Deleted %s", m_fileToDelete.cstr());
+				HighRezTimer::sleep(1.0_ms);
 			}
 		}
 	}
@@ -77,15 +78,14 @@ static CString profile(ShaderType shaderType)
 Error compileHlslToSpirv(CString src, ShaderType shaderType, BaseMemoryPool& tmpPool, DynamicArrayRaii<U8>& spirv,
 						 StringRaii& errorMessage)
 {
-	srand(U32(time(0)));
-	const I32 r = rand();
+	const U64 rand = getRandom();
 
 	StringRaii tmpDir(&tmpPool);
 	ANKI_CHECK(getTempDirectory(tmpDir));
 
-	// Store src to a file
+	// Store HLSL to a file
 	StringRaii hlslFilename(&tmpPool);
-	hlslFilename.sprintf("%s/%d.hlsl", tmpDir.cstr(), r);
+	hlslFilename.sprintf("%s/%llu.hlsl", tmpDir.cstr(), rand);
 
 	File hlslFile;
 	ANKI_CHECK(hlslFile.open(hlslFilename, FileOpenFlag::kWrite));
@@ -95,9 +95,11 @@ Error compileHlslToSpirv(CString src, ShaderType shaderType, BaseMemoryPool& tmp
 
 	// Call DXC
 	StringRaii spvFilename(&tmpPool);
-	spvFilename.sprintf("%s/%d.spv", tmpDir.cstr(), r);
+	spvFilename.sprintf("%s/%llu.spv", tmpDir.cstr(), rand);
 
 	DynamicArrayRaii<StringRaii> dxcArgs(&tmpPool);
+	dxcArgs.emplaceBack(&tmpPool, "-Fo");
+	dxcArgs.emplaceBack(&tmpPool, spvFilename);
 	dxcArgs.emplaceBack(&tmpPool, "-Wall");
 	dxcArgs.emplaceBack(&tmpPool, "-Wextra");
 	dxcArgs.emplaceBack(&tmpPool, "-Wconversion");
@@ -109,8 +111,6 @@ Error compileHlslToSpirv(CString src, ShaderType shaderType, BaseMemoryPool& tmp
 	dxcArgs.emplaceBack(&tmpPool, "2021");
 	dxcArgs.emplaceBack(&tmpPool, "-E");
 	dxcArgs.emplaceBack(&tmpPool, "main");
-	dxcArgs.emplaceBack(&tmpPool, "-Fo");
-	dxcArgs.emplaceBack(&tmpPool, spvFilename);
 	dxcArgs.emplaceBack(&tmpPool, "-T");
 	dxcArgs.emplaceBack(&tmpPool, profile(shaderType));
 	dxcArgs.emplaceBack(&tmpPool, "-spirv");
@@ -123,26 +123,42 @@ Error compileHlslToSpirv(CString src, ShaderType shaderType, BaseMemoryPool& tmp
 		dxcArgs2[i] = dxcArgs[i];
 	}
 
-	Process proc;
-	ANKI_CHECK(proc.start(ANKI_SOURCE_DIRECTORY "/ThirdParty/Bin/Windows64/dxc.exe", dxcArgs2));
+	while(true)
+	{
+		I32 exitCode;
+		StringRaii stdOut(&tmpPool);
 
-	I32 exitCode;
-	ProcessStatus status;
-	ANKI_CHECK(proc.wait(60.0_sec, &status, &exitCode));
+		// Run once without stdout or stderr. Because if you do the process library will crap out after a while
+		ANKI_CHECK(Process::callProcess(ANKI_SOURCE_DIRECTORY "/ThirdParty/Bin/Windows64/dxc.exe", dxcArgs2, nullptr,
+										nullptr, exitCode));
 
-	if(!(status == ProcessStatus::kNotRunning && exitCode == 0))
-	{
 		if(exitCode != 0)
 		{
-			ANKI_CHECK(proc.readFromStderr(errorMessage));
-		}
+			// There was an error, run again just to get the stderr
+			ANKI_CHECK(Process::callProcess(ANKI_SOURCE_DIRECTORY "/ThirdParty/Bin/Windows64/dxc.exe", dxcArgs2,
+											nullptr, &errorMessage, exitCode));
 
-		if(errorMessage.isEmpty())
+			if(!errorMessage.isEmpty()
+			   && errorMessage.find("The process cannot access the file because") != CString::kNpos)
+			{
+				// DXC might fail to read the HLSL because the antivirus might be having a lock on it. Try again
+				errorMessage.destroy();
+				HighRezTimer::sleep(1.0_ms);
+			}
+			else if(errorMessage.isEmpty())
+			{
+				errorMessage = "Unknown error";
+				return Error::kFunctionFailed;
+			}
+			else
+			{
+				return Error::kFunctionFailed;
+			}
+		}
+		else
 		{
-			errorMessage = "Unknown error";
+			break;
 		}
-
-		return Error::kFunctionFailed;
 	}
 
 	CleanupFile spvFileCleanup(&tmpPool, spvFilename);
@@ -152,6 +168,7 @@ Error compileHlslToSpirv(CString src, ShaderType shaderType, BaseMemoryPool& tmp
 	ANKI_CHECK(spvFile.open(spvFilename, FileOpenFlag::kRead));
 	spirv.resize(U32(spvFile.getSize()));
 	ANKI_CHECK(spvFile.read(&spirv[0], spirv.getSizeInBytes()));
+	spvFile.close();
 
 	return Error::kNone;
 }

+ 2 - 1
AnKi/Util/Filesystem.cpp

@@ -75,7 +75,8 @@ Error removeFile(const CString& filename)
 	const int err = std::remove(filename.cstr());
 	if(err)
 	{
-		ANKI_UTIL_LOGE("Couldn't delete file: %s", filename.cstr());
+		ANKI_UTIL_LOGE("Couldn't delete file (%s): %s", strerror(errno), filename.cstr());
+		return Error::kFunctionFailed;
 	}
 
 	return Error::kNone;

+ 48 - 7
AnKi/Util/Process.cpp

@@ -5,6 +5,7 @@
 
 #include <AnKi/Util/Process.h>
 #include <AnKi/Util/Array.h>
+#include <AnKi/Util/Thread.h>
 #if !ANKI_OS_ANDROID
 #	include <ThirdParty/Reproc/reproc/include/reproc/reproc.h>
 #endif
@@ -33,7 +34,8 @@ void Process::destroy()
 #endif
 }
 
-Error Process::start(CString executable, ConstWeakArray<CString> arguments, ConstWeakArray<CString> environment)
+Error Process::start(CString executable, ConstWeakArray<CString> arguments, ConstWeakArray<CString> environment,
+					 ProcessOptions options)
 {
 #if !ANKI_OS_ANDROID
 	ANKI_ASSERT(m_handle == nullptr && "Already started");
@@ -60,18 +62,22 @@ Error Process::start(CString executable, ConstWeakArray<CString> arguments, Cons
 	// Start process
 	m_handle = reproc_new();
 
-	reproc_options options = {};
+	reproc_options reprocOptions = {};
 
-	options.env.behavior = REPROC_ENV_EXTEND;
+	reprocOptions.env.behavior = REPROC_ENV_EXTEND;
 	if(environment.getSize())
 	{
-		options.env.extra = &env[0];
+		reprocOptions.env.extra = &env[0];
 	}
-	options.nonblocking = true;
+	reprocOptions.nonblocking = true;
 
-	options.redirect.err.type = REPROC_REDIRECT_PIPE;
+	reprocOptions.redirect.in.type = REPROC_REDIRECT_DISCARD;
+	reprocOptions.redirect.err.type =
+		(!!(options & ProcessOptions::kOpenStderr)) ? REPROC_REDIRECT_PIPE : REPROC_REDIRECT_DISCARD;
+	reprocOptions.redirect.out.type =
+		(!!(options & ProcessOptions::kOpenStdout)) ? REPROC_REDIRECT_PIPE : REPROC_REDIRECT_DISCARD;
 
-	I32 ret = reproc_start(m_handle, &args[0], options);
+	I32 ret = reproc_start(m_handle, &args[0], reprocOptions);
 	if(ret < 0)
 	{
 		ANKI_UTIL_LOGE("reproc_start() failed: %s", reproc_strerror(ret));
@@ -236,4 +242,39 @@ Error Process::readCommon(I32 reprocStream, StringRaii& text)
 }
 #endif
 
+Error Process::callProcess(CString executable, ConstWeakArray<CString> arguments, StringRaii* stdOut,
+						   StringRaii* stdErr, I32& exitCode)
+{
+#if !ANKI_OS_ANDROID
+	ProcessOptions options = ProcessOptions::kNone;
+	options |= (stdOut) ? ProcessOptions::kOpenStdout : ProcessOptions::kNone;
+	options |= (stdErr) ? ProcessOptions::kOpenStderr : ProcessOptions::kNone;
+
+	Process proc;
+	ANKI_CHECK(proc.start(executable, arguments, {}, options));
+
+	ANKI_CHECK(proc.wait(-1.0, nullptr, &exitCode));
+
+	if(stdOut)
+	{
+		ANKI_CHECK(proc.readFromStdout(*stdOut));
+	}
+
+	if(stdErr)
+	{
+		ANKI_CHECK(proc.readFromStderr(*stdErr));
+	}
+
+	proc.destroy();
+#else
+	(void)executable;
+	(void)arguments;
+	(void)stdOut;
+	(void)stdErr;
+	(void)exitCode;
+#endif
+
+	return Error::kNone;
+}
+
 } // end namespace anki

+ 21 - 1
AnKi/Util/Process.h

@@ -8,6 +8,7 @@
 #include <AnKi/Util/Array.h>
 #include <AnKi/Util/String.h>
 #include <AnKi/Util/WeakArray.h>
+#include <AnKi/Util/Enum.h>
 
 // Forward
 struct reproc_t;
@@ -31,6 +32,15 @@ enum class ProcessKillSignal : U8
 	kForce
 };
 
+/// @memberof Process
+enum class ProcessOptions : U8
+{
+	kNone = 0,
+	kOpenStdout = 1 << 0,
+	kOpenStderr = 1 << 1,
+};
+ANKI_ENUM_ALLOW_NUMERIC_OPERATIONS(ProcessOptions)
+
 /// Executes external processes.
 class Process
 {
@@ -47,7 +57,8 @@ public:
 	/// @param executable The executable to start.
 	/// @param arguments The command line arguments.
 	/// @param environment The environment variables.
-	Error start(CString executable, ConstWeakArray<CString> arguments = {}, ConstWeakArray<CString> environment = {});
+	Error start(CString executable, ConstWeakArray<CString> arguments = {}, ConstWeakArray<CString> environment = {},
+				ProcessOptions options = ProcessOptions::kOpenStderr | ProcessOptions::kOpenStdout);
 
 	/// Wait for the process to finish.
 	/// @param timeout The time to wait. If it's negative wait forever.
@@ -71,6 +82,15 @@ public:
 	/// calling destroy.
 	void destroy();
 
+	/// Call a process and wait.
+	/// @param executable The executable to start.
+	/// @param arguments The command line arguments.
+	/// @param stdOut Optional stdout.
+	/// @param stdErr Optional stderr.
+	/// @param exitCode Exit code.
+	static Error callProcess(CString executable, ConstWeakArray<CString> arguments, StringRaii* stdOut,
+							 StringRaii* stdErr, I32& exitCode);
+
 private:
 	reproc_t* m_handle = nullptr;