Add thread-safe double-checked locking to MotelyPaths.Initialize

[Copilot]

The volatile keyword alone is insufficient for thread-safe initialization. Multiple threads calling Initialize concurrently could race, creating directories multiple times or setting _isInitialized before initialization completes.

Changes

• Added lock-based synchronization: Introduced _initLock with double-checked locking pattern
• Fast path optimization: Check _isInitialized before acquiring lock to minimize contention post-initialization
• Atomic initialization: All field assignments and directory creation complete within lock before marking initialized
• Fail-safe behavior: Exceptions during directory creation prevent _isInitialized from being set, allowing retry

public static void Initialize(IWebHostEnvironment env, IConfiguration? config = null)
{
    // Fast path: if already initialized, return immediately
    if (_isInitialized)
        return;

    lock (_initLock)
    {
        // Double-check after acquiring lock
        if (_isInitialized)
            return;

        _contentRoot = env.ContentRootPath;
        // ... set overrides, create directories ...
        _isInitialized = true;
    }
}

The volatile fields remain necessary for memory ordering guarantees on the initialization flag.

---

💡 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.

[Copilot]

Pull request overview

This PR implements thread-safe initialization for the MotelyPaths static class using a double-checked locking pattern to address concurrency concerns raised in code review feedback.

Changes:

• Added thread-safe initialization using double-checked locking pattern with a dedicated lock object
• Updated documentation to clarify thread-safety guarantees and behavior
• Ensured atomic initialization where directory creation failures allow for retry

Comments suppressed due to low confidence (1)

Motely.API/MotelyPaths.cs:18

• The volatile keyword on the string fields (_contentRoot, _jamlFiltersOverride, etc.) is unnecessary and potentially misleading. In the double-checked locking pattern, only the _isInitialized flag needs to be volatile to ensure proper memory ordering. The lock in the Initialize method already provides the necessary memory barriers to ensure that all field writes inside the lock are visible to threads that read _isInitialized as true. These volatile keywords can be removed from the string fields while keeping it only on _isInitialized.

    private static volatile string _contentRoot = Directory.GetCurrentDirectory();
    private static volatile string? _jamlFiltersOverride;
    private static volatile string? _seedSourcesOverride;
    private static volatile string? _searchResultsOverride;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The comment states "Kept inside the lock to ensure atomic initialization" but the actual reason is more nuanced. Directory creation is a side effect that can fail, and keeping it inside the lock ensures that if it fails and throws an exception, _isInitialized remains false, allowing retry. However, the operations themselves are not truly "atomic" - if directory creation partially succeeds then fails, some directories may exist while others don't. Consider clarifying this comment to explain the retry-on-failure behavior more explicitly.

Suggested change

	// Kept inside the lock to ensure atomic initialization: if directory creation fails,
	// _isInitialized remains false, allowing retry on next Initialize call.
	// Kept inside the lock so that if any directory creation fails and throws,
	// _isInitialized remains false and a future Initialize call can safely retry.