From cfc81c9ad44f292fda960693c230a01d6249667c Mon Sep 17 00:00:00 2001 From: Richard Stanway Date: Wed, 10 Jun 2026 22:59:50 +0200 Subject: [PATCH] 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. --- libobs-d3d11/d3d11-shader.cpp | 104 ++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 42 deletions(-) diff --git a/libobs-d3d11/d3d11-shader.cpp b/libobs-d3d11/d3d11-shader.cpp index 930062b09..76321cdd9 100644 --- a/libobs-d3d11/d3d11-shader.cpp +++ b/libobs-d3d11/d3d11-shader.cpp @@ -27,6 +27,21 @@ #include #include #include +#include + +namespace { + +void removeCacheFile(const std::filesystem::path &cachePath, const char *reason) noexcept +{ + blog(LOG_WARNING, "Discarding shader cache file %s: %s", + reinterpret_cast(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 &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(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((*shader)->GetBufferPointer()), len); + cacheFile.read(reinterpret_cast(&checksum), sizeof(checksum)); + + const bool success = static_cast(cacheFile); + + if (success) { + uint64_t calculated_checksum = + fnv1a_hash(static_cast((*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((*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((*shader)->GetBufferPointer()), (*shader)->GetBufferSize()); + outFile.write(reinterpret_cast(&calculated_checksum), sizeof(calculated_checksum)); + outFile.close(); + + if (outFile.fail()) { + removeCacheFile(cachePath, "write error"); } } }