From a697c7a00ddefd42b1ca575beebb7b9d01ac4fd2 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 | 94 ++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 39 deletions(-) diff --git a/libobs-d3d11/d3d11-shader.cpp b/libobs-d3d11/d3d11-shader.cpp index 930062b099b49f..6121085a8c469e 100644 --- a/libobs-d3d11/d3d11-shader.cpp +++ b/libobs-d3d11/d3d11-shader.cpp @@ -27,6 +27,7 @@ #include #include #include +#include void gs_vertex_shader::GetBuffersExpected(const std::vector &inputs) { @@ -218,6 +219,16 @@ static uint64_t fnv1a_hash(const char *str, size_t len) return hash; } +static void remove_cache_file(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); +} + void gs_shader::Compile(const char *shaderString, const char *file, const char *target, ID3D10Blob **shader) { ComPtr errorsBlob; @@ -239,41 +250,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); + std::ifstream cacheFile(cachePath, std::ios::binary | std::ios::ate); + if (cacheFile.is_open()) { + uint64_t checksum = 0; - if (std::filesystem::exists(cachePath) && !std::filesystem::is_empty(cachePath)) { - cacheFile.open(cachePath, std::ios::in | std::ios::binary | std::ios::ate); - } + std::streamoff len = cacheFile.tellg(); - if (cacheFile.is_open()) { - uint64_t checksum; + // Not enough data for checksum + shader + if (len < 0 || len <= static_cast(sizeof(checksum))) { + cacheFile.close(); + remove_cache_file(cachePath, "truncated or unreadable"); + } else { + len -= sizeof(checksum); - try { - std::streampos len = cacheFile.tellg(); - // Not enough data for checksum + shader - if (len <= sizeof(checksum)) { - throw std::length_error("File truncated"); - } + hr = D3DCreateBlob(len, shader); + if (FAILED(hr)) { + cacheFile.close(); + remove_cache_file(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)); - cacheFile.seekg(0, std::ios::beg); + const bool success = static_cast(cacheFile); - len -= sizeof(checksum); - D3DCreateBlob(len, shader); - cacheFile.read((char *)(*shader)->GetBufferPointer(), len); - uint64_t calculated_checksum = fnv1a_hash((char *)(*shader)->GetBufferPointer(), len); + if (success) { + uint64_t calculated_checksum = + fnv1a_hash(static_cast((*shader)->GetBufferPointer()), len); - cacheFile.read((char *)&checksum, sizeof(checksum)); - if (calculated_checksum != checksum) { - throw std::exception("Checksum mismatch"); - } + if (calculated_checksum == checksum) { + is_cached = true; + } + } - 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); - cacheFile.close(); - std::filesystem::remove(cachePath); + if (!is_cached) { + (*shader)->Release(); + *shader = nullptr; + + cacheFile.close(); + remove_cache_file(cachePath, !success ? "read error" : "checksum mismatch"); + } + } } } @@ -288,18 +305,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()) { + remove_cache_file(cachePath, "write error"); } } }