Repository navigation
Fix memory manager data races - a follow up fix - #1794
Merged
Merged
Conversation
aleksandr-voitenko
force-pushed
the
fix-memory-manager-data-races-2
branch
from
September 29, 2026 20:44
104ee1b to
61bda66
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A cache-setting job can still overlap a guarded file change and enable caching using a stale reservation.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds guarded source-setting updates to prevent stale media metadata from corrupting cache reservations.
Changes:
- Introduces scoped settings-update guards and deferred cache queries.
- Applies guards to source updates and property callbacks.
- Adds regression tests and API timing documentation.
| File | Description |
|---|---|
obs-studio-server/source/memory-manager.cpp |
Implements guard tracking and delayed graphics queries. |
obs-studio-server/source/memory-manager.h |
Defines the guard API and lifecycle contract. |
obs-studio-server/source/osn-source.cpp |
Guards source update entry points. |
obs-studio-server/tests/test-memory-manager.cpp |
Adds settings-race and reset coverage. |
js/module.ts |
Documents asynchronous video-source updates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
summeroff
approved these changes
Sep 29, 2026
sandboxcoder
approved these changes
Sep 29, 2026
summeroff
approved these changes
Sep 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add a scoped settings-update guard around
Source::UpdateandSource::GetProperties, including property callbacks. The guard invalidates existing work before settings change and delays queries until all updates finish and an OBS source-update phase has passed. Delayed queries remain cancellable without consuming readiness retries or blocking other sources.Add six regression cases covering deferred updates, overlapping updates, stale query results, property callbacks, source registration identity, and shutdown. Extend graphics-reset coverage and document the update timing and guard lifetime.
Motivation and Context
Changing a media file updates its settings before OBS applies the deferred plugin update. During that interval, the cache manager can associate the new filename with the previous player's metadata and enable caching with an incorrect memory reservation.
How Has This Been Tested?
Tests, Windows only.
Types of changes