mirror of
https://github.com/obsproject/obs-studio.git
synced 2026-09-26 01:10:38 -04:00
libobs-d3d11: Avoid exceptions in shader cache file handling
We're still getting crash reports from this code and it seems like C++ file I/O with exceptions set is a minefield - even closing a file in an exception handler can trigger further exceptions from buffer flushes for example. Using std::filesystem to check the file exists before opening it also introduces exceptions and is a pointless TOCTOU check anyway. This commit removes the exception bits from the streams and relies on ifstream operator bool and ofstream fail() instead, greatly reducing the number of possible exception generating paths we need to worry about.
This commit is contained in:
committed by
Ryan Foster
parent
617f7ed707
commit
cfc81c9ad4
@@ -27,6 +27,21 @@
|
||||
#include <filesystem>
|
||||
#include <fstream>
|
||||
#include <d3dcompiler.h>
|
||||
#include <system_error>
|
||||
|
||||
namespace {
|
||||
|
||||
void removeCacheFile(const std::filesystem::path &cachePath, const char *reason) noexcept
|
||||
{
|
||||
blog(LOG_WARNING, "Discarding shader cache file %s: %s",
|
||||
reinterpret_cast<const char *>(cachePath.u8string().c_str()), reason);
|
||||
|
||||
// Intentionally ignored - we don't care about failure here, we just don't want exceptions
|
||||
std::error_code ec;
|
||||
std::filesystem::remove(cachePath, ec);
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
void gs_vertex_shader::GetBuffersExpected(const std::vector<D3D11_INPUT_ELEMENT_DESC> &inputs)
|
||||
{
|
||||
@@ -239,41 +254,47 @@ void gs_shader::Compile(const char *shaderString, const char *file, const char *
|
||||
// Increment if on-disk format changes
|
||||
cachePath += ".v2";
|
||||
|
||||
std::fstream cacheFile;
|
||||
cacheFile.exceptions(std::fstream::badbit | std::fstream::eofbit);
|
||||
|
||||
if (std::filesystem::exists(cachePath) && !std::filesystem::is_empty(cachePath)) {
|
||||
cacheFile.open(cachePath, std::ios::in | std::ios::binary | std::ios::ate);
|
||||
}
|
||||
|
||||
std::ifstream cacheFile{cachePath, std::ios::binary | std::ios::ate};
|
||||
if (cacheFile.is_open()) {
|
||||
uint64_t checksum;
|
||||
uint64_t checksum = 0;
|
||||
|
||||
try {
|
||||
std::streampos len = cacheFile.tellg();
|
||||
// Not enough data for checksum + shader
|
||||
if (len <= sizeof(checksum)) {
|
||||
throw std::length_error("File truncated");
|
||||
}
|
||||
std::streamoff len = cacheFile.tellg();
|
||||
|
||||
cacheFile.seekg(0, std::ios::beg);
|
||||
|
||||
len -= sizeof(checksum);
|
||||
D3DCreateBlob(len, shader);
|
||||
cacheFile.read((char *)(*shader)->GetBufferPointer(), len);
|
||||
uint64_t calculated_checksum = fnv1a_hash((char *)(*shader)->GetBufferPointer(), len);
|
||||
|
||||
cacheFile.read((char *)&checksum, sizeof(checksum));
|
||||
if (calculated_checksum != checksum) {
|
||||
throw std::exception("Checksum mismatch");
|
||||
}
|
||||
|
||||
is_cached = true;
|
||||
} catch (const std::exception &e) {
|
||||
// Something went wrong reading the cache file, delete it
|
||||
blog(LOG_WARNING, "Loading shader cache file failed with \"%s\": %s", e.what(), file);
|
||||
// Not enough data for checksum + shader
|
||||
if (len < 0 || len <= static_cast<std::streamoff>(sizeof(checksum))) {
|
||||
cacheFile.close();
|
||||
std::filesystem::remove(cachePath);
|
||||
removeCacheFile(cachePath, "truncated or unreadable");
|
||||
} else {
|
||||
len -= sizeof(checksum);
|
||||
|
||||
hr = D3DCreateBlob(len, shader);
|
||||
if (FAILED(hr)) {
|
||||
cacheFile.close();
|
||||
removeCacheFile(cachePath, "cache blob allocation failed");
|
||||
} else {
|
||||
cacheFile.seekg(0, std::ios::beg);
|
||||
cacheFile.read(static_cast<char *>((*shader)->GetBufferPointer()), len);
|
||||
cacheFile.read(reinterpret_cast<char *>(&checksum), sizeof(checksum));
|
||||
|
||||
const bool success = static_cast<bool>(cacheFile);
|
||||
|
||||
if (success) {
|
||||
uint64_t calculated_checksum =
|
||||
fnv1a_hash(static_cast<char *>((*shader)->GetBufferPointer()), len);
|
||||
|
||||
if (calculated_checksum == checksum) {
|
||||
is_cached = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (!is_cached) {
|
||||
(*shader)->Release();
|
||||
*shader = nullptr;
|
||||
|
||||
cacheFile.close();
|
||||
removeCacheFile(cachePath, !success ? "read error" : "checksum mismatch");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -288,18 +309,17 @@ void gs_shader::Compile(const char *shaderString, const char *file, const char *
|
||||
}
|
||||
}
|
||||
|
||||
cacheFile.open(cachePath, std::ios::out | std::ios::binary);
|
||||
if (cacheFile.is_open()) {
|
||||
try {
|
||||
uint64_t calculated_checksum =
|
||||
fnv1a_hash((char *)(*shader)->GetBufferPointer(), (*shader)->GetBufferSize());
|
||||
std::ofstream outFile{cachePath, std::ios::binary | std::ios::trunc};
|
||||
if (outFile.is_open()) {
|
||||
uint64_t calculated_checksum = fnv1a_hash(static_cast<char *>((*shader)->GetBufferPointer()),
|
||||
(*shader)->GetBufferSize());
|
||||
|
||||
cacheFile.write((char *)(*shader)->GetBufferPointer(), (*shader)->GetBufferSize());
|
||||
cacheFile.write((char *)&calculated_checksum, sizeof(calculated_checksum));
|
||||
} catch (const std::exception &e) {
|
||||
blog(LOG_WARNING, "Writing shader cache file failed with \"%s\": %s", e.what(), file);
|
||||
cacheFile.close();
|
||||
std::filesystem::remove(cachePath);
|
||||
outFile.write(static_cast<char *>((*shader)->GetBufferPointer()), (*shader)->GetBufferSize());
|
||||
outFile.write(reinterpret_cast<char *>(&calculated_checksum), sizeof(calculated_checksum));
|
||||
outFile.close();
|
||||
|
||||
if (outFile.fail()) {
|
||||
removeCacheFile(cachePath, "write error");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user