Skip to content

libobs-d3d11: Avoid exceptions in shader cache file handling - #13538

Merged
RytoEX merged 1 commit into
obsproject:masterfrom
notr1ch:shader-cache-noexcept
Sep 18, 2026
Merged

RytoEX merged 1 commit into
obsproject:masterfrom
notr1ch:shader-cache-noexcept

Conversation

@notr1ch

@notr1ch notr1ch commented Jun 10, 2026

Copy link
Copy Markdown
Member

Description

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.

Motivation and Context

Fix reported crashes.

How Has This Been Tested?

Tested with read only, deleted files, corrupt files as much as possible.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have read the contributing document.
  • My code has been run through clang-format.
  • My code follows the project's style guidelines
  • My code is not on the master branch.
  • My code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@notr1ch notr1ch added kind/bug Categorizes issue or PR as related to a bug. platform/windows Categorizes issue or PR as affecting Windows specifically labels Jun 10, 2026
@github-project-automation github-project-automation Bot moved this to Ready For Review in 33.0 Release Tracker Jul 6, 2026
@Warchamp7 Warchamp7 added this to the OBS Studio 33.0 milestone Aug 7, 2026
@RytoEX
RytoEX requested a review from PatTheMav August 20, 2026 22:53

@PatTheMav PatTheMav left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just code style nits, seems fine otherwise.

Comment thread libobs-d3d11/d3d11-shader.cpp Outdated
Comment thread libobs-d3d11/d3d11-shader.cpp Outdated
Comment thread libobs-d3d11/d3d11-shader.cpp Outdated
@github-project-automation github-project-automation Bot moved this from Ready For Review to Requires Changes in 33.0 Release Tracker Aug 21, 2026
@notr1ch
notr1ch force-pushed the shader-cache-noexcept branch from a697c7a to 989b203 Compare August 21, 2026 20:16

@PatTheMav PatTheMav left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave it to @RytoEX whether we want the last code style nit to be fixed, change is approved regardless.

Comment thread libobs-d3d11/d3d11-shader.cpp Outdated
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.
@RytoEX
RytoEX force-pushed the shader-cache-noexcept branch from 989b203 to bb586ac Compare September 18, 2026 20:19
@RytoEX
RytoEX merged commit cfc81c9 into obsproject:master Sep 18, 2026
15 checks passed
@github-project-automation github-project-automation Bot moved this from Requires Changes to Merged in 33.0 Release Tracker Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Categorizes issue or PR as related to a bug. kind/crash platform/windows Categorizes issue or PR as affecting Windows specifically

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

4 participants