Fix memory ordering race condition in MotelyPaths initialization#16
Conversation
…ce condition Co-authored-by: joirunner <6373131+joirunner@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a memory ordering race condition in the MotelyPaths static class by adding the volatile keyword to three static fields that store path override configurations. The fix ensures proper memory visibility when these fields are written during initialization and subsequently read by concurrent threads accessing path properties.
Changes:
- Added
volatilemodifier to_jamlFiltersOverride,_seedSourcesOverride, and_searchResultsOverridestatic fields to ensure memory ordering consistency
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static volatile string? _jamlFiltersOverride; | ||
| private static volatile string? _seedSourcesOverride; | ||
| private static volatile string? _searchResultsOverride; |
There was a problem hiding this comment.
The MotelyPaths class lacks test coverage for its initialization and thread-safety behavior. Consider adding tests to verify that the volatile fields are properly initialized and that concurrent access to path properties after initialization works correctly. This is especially important given that this PR addresses a memory ordering race condition.
The
MotelyPathsstatic class had a memory ordering race condition: only_contentRootand_isInitializedwere volatile, but the override path fields were not. Threads could observe_isInitialized = truewhile still seeing uninitialized values for_jamlFiltersOverride,_seedSourcesOverride, and_searchResultsOverridedue to CPU cache coherency delays.Changes
MotelyPathsvolatile to ensure proper memory orderingThis guarantees that when a thread observes
_isInitialized = true, it will also observe the correct initialized values for all override paths written duringInitialize().💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.