Add ScopedEnvironmentVar class for setting env variables in BackendVk
In the Vulkan backend, we only need to set VK_LAYER_PATH and
VK_ICD_FILENAMES while we're gathering the device information. After
this, we should unset the environment variables or they will persist
and affect loading Vulkan again (from Dawn or another client) in the
future.
Bug: dawn:406
Change-Id: I30c38b0980e181126fcd7fa911bbf9e8aa35f3b8
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22021
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/src/common/SystemUtils.cpp b/src/common/SystemUtils.cpp
index 73aa4ee..f8282eb 100644
--- a/src/common/SystemUtils.cpp
+++ b/src/common/SystemUtils.cpp
@@ -14,6 +14,8 @@
#include "common/SystemUtils.h"
+#include "common/Assert.h"
+
#if defined(DAWN_PLATFORM_WINDOWS)
# include <Windows.h>
# include <vector>
@@ -115,3 +117,27 @@
size_t lastPathSepLoc = exePath.find_last_of(GetPathSeparator());
return lastPathSepLoc != std::string::npos ? exePath.substr(0, lastPathSepLoc + 1) : "";
}
+
+// ScopedEnvironmentVar
+
+ScopedEnvironmentVar::ScopedEnvironmentVar(const char* variableName, const char* value)
+ : mName(variableName),
+ mOriginalValue(GetEnvironmentVar(variableName)),
+ mIsSet(SetEnvironmentVar(variableName, value)) {
+}
+
+ScopedEnvironmentVar::~ScopedEnvironmentVar() {
+ if (mIsSet) {
+ bool success = SetEnvironmentVar(mName.c_str(), mOriginalValue.c_str());
+ // If we set the environment variable in the constructor, we should never fail restoring it.
+ ASSERT(success);
+ }
+}
+
+bool ScopedEnvironmentVar::Set(const char* variableName, const char* value) {
+ ASSERT(!mIsSet);
+ mName = variableName;
+ mOriginalValue = GetEnvironmentVar(variableName);
+ mIsSet = SetEnvironmentVar(variableName, value);
+ return mIsSet;
+}
diff --git a/src/common/SystemUtils.h b/src/common/SystemUtils.h
index 2edf1e3..ed18c31 100644
--- a/src/common/SystemUtils.h
+++ b/src/common/SystemUtils.h
@@ -24,4 +24,21 @@
bool SetEnvironmentVar(const char* variableName, const char* value);
std::string GetExecutableDirectory();
+class ScopedEnvironmentVar {
+ public:
+ ScopedEnvironmentVar() = default;
+ ScopedEnvironmentVar(const char* variableName, const char* value);
+ ~ScopedEnvironmentVar();
+
+ ScopedEnvironmentVar(const ScopedEnvironmentVar& rhs) = delete;
+ ScopedEnvironmentVar& operator=(const ScopedEnvironmentVar& rhs) = delete;
+
+ bool Set(const char* variableName, const char* value);
+
+ private:
+ std::string mName;
+ std::string mOriginalValue;
+ bool mIsSet = false;
+};
+
#endif // COMMON_SYSTEMUTILS_H_
diff --git a/src/dawn_native/vulkan/BackendVk.cpp b/src/dawn_native/vulkan/BackendVk.cpp
index eef62e8..fe9eba6 100644
--- a/src/dawn_native/vulkan/BackendVk.cpp
+++ b/src/dawn_native/vulkan/BackendVk.cpp
@@ -109,14 +109,16 @@
MaybeError Backend::Initialize(bool useSwiftshader) {
DAWN_TRY(LoadVulkan(useSwiftshader));
- // TODO(crbug.com/dawn/406): In order to not modify the environment variables of
- // the rest of an application embedding Dawn, we should set these only
- // in the scope of this function. See ANGLE's ScopedVkLoaderEnvironment
+ // These environment variables need only be set while loading procs and gathering device
+ // info.
+ ScopedEnvironmentVar vkICDFilenames;
+ ScopedEnvironmentVar vkLayerPath;
+
if (useSwiftshader) {
#if defined(DAWN_SWIFTSHADER_VK_ICD_JSON)
std::string fullSwiftshaderICDPath =
GetExecutableDirectory() + DAWN_SWIFTSHADER_VK_ICD_JSON;
- if (!SetEnvironmentVar("VK_ICD_FILENAMES", fullSwiftshaderICDPath.c_str())) {
+ if (!vkICDFilenames.Set("VK_ICD_FILENAMES", fullSwiftshaderICDPath.c_str())) {
return DAWN_INTERNAL_ERROR("Couldn't set VK_ICD_FILENAMES");
}
#else
@@ -128,7 +130,7 @@
if (GetInstance()->IsBackendValidationEnabled()) {
#if defined(DAWN_ENABLE_VULKAN_VALIDATION_LAYERS)
std::string vkDataDir = GetExecutableDirectory() + DAWN_VK_DATA_DIR;
- if (!SetEnvironmentVar("VK_LAYER_PATH", vkDataDir.c_str())) {
+ if (!vkLayerPath.Set("VK_LAYER_PATH", vkDataDir.c_str())) {
return DAWN_INTERNAL_ERROR("Couldn't set VK_LAYER_PATH");
}
#else
diff --git a/src/tests/unittests/SystemUtilsTests.cpp b/src/tests/unittests/SystemUtilsTests.cpp
index 540f4ef..3730f45 100644
--- a/src/tests/unittests/SystemUtilsTests.cpp
+++ b/src/tests/unittests/SystemUtilsTests.cpp
@@ -39,3 +39,48 @@
// Test last charecter in path
ASSERT_EQ(GetExecutableDirectory().back(), *GetPathSeparator());
}
+
+// Tests for ScopedEnvironmentVar
+TEST(SystemUtilsTests, ScopedEnvironmentVar) {
+ SetEnvironmentVar("ScopedEnvironmentVarForTest", "original");
+
+ // Test empty environment variable doesn't crash
+ { ScopedEnvironmentVar var; }
+
+ // Test setting empty environment variable
+ {
+ ScopedEnvironmentVar var;
+ var.Set("ScopedEnvironmentVarForTest", "NewEnvironmentVarValue");
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "NewEnvironmentVarValue");
+ }
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
+
+ // Test that the environment variable can be set, and it is unset at the end of the scope.
+ {
+ ScopedEnvironmentVar var("ScopedEnvironmentVarForTest", "NewEnvironmentVarValue");
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "NewEnvironmentVarValue");
+ }
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
+
+ // Test nested scopes
+ {
+ ScopedEnvironmentVar outer("ScopedEnvironmentVarForTest", "outer");
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "outer");
+ {
+ ScopedEnvironmentVar inner("ScopedEnvironmentVarForTest", "inner");
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "inner");
+ }
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "outer");
+ }
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
+
+ // Test redundantly setting scoped variables
+ {
+ ScopedEnvironmentVar var1("ScopedEnvironmentVarForTest", "var1");
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "var1");
+
+ ScopedEnvironmentVar var2("ScopedEnvironmentVarForTest", "var2");
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "var2");
+ }
+ ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
+}
