Skip to content

Conversation

@xezon
Copy link

@xezon xezon commented Jan 26, 2026

This change merges code of CriticalSection and ScopedMutex.

I decided to copy from Generals to Zero Hour, because that makes more sense.

Zero Hour gets

  • ScopedMutex no longer has a timeout of 500 ms. It seems counter productive to time out mutexes.

@xezon xezon added this to the Code foundation build up milestone Jan 26, 2026
@xezon xezon added ZH Relates to Zero Hour Unify Unifies code between Generals and Zero Hour labels Jan 26, 2026
@xezon xezon changed the title unify(common): Merge CriticalSection, ScopedMutex code unify(common): Merge CriticalSection, ScopedMutex code from Generals Jan 26, 2026
@greptile-apps
Copy link

greptile-apps bot commented Jan 26, 2026

Greptile Overview

Greptile Summary

Unified the CriticalSection and ScopedMutex implementations between Generals and Zero Hour by copying from Generals to Zero Hour (GeneralsMD).

Key Changes:

  • Removed transitive #include "mutex.h" from CriticalSection.h - this include was at the end of the file and provided classes like MutexClass, CriticalSectionClass, and FastCriticalSectionClass to files that included CriticalSection.h. Analysis confirms only W3DMouse.cpp uses these classes and it already includes mutex.h directly.
  • Changed ScopedMutex from using a 500ms timeout with DEBUG_LOG error handling to using INFINITE wait - this is a more correct approach as the previous implementation would proceed with execution even if the lock acquisition failed, potentially causing race conditions.

Confidence Score: 5/5

  • This PR is safe to merge with no issues found
  • The changes are well-contained and improve code correctness. Removal of the transitive include is safe (verified only W3DMouse.cpp uses mutex.h classes and includes it directly). The timeout removal prevents potential race conditions from proceeding with failed lock acquisitions.
  • No files require special attention

Important Files Changed

Filename Overview
GeneralsMD/Code/GameEngine/Include/Common/CriticalSection.h Removed transitive include of mutex.h which is no longer needed
GeneralsMD/Code/GameEngine/Include/Common/ScopedMutex.h Changed mutex wait from 500ms timeout to INFINITE, removing defensive logging

Sequence Diagram

sequenceDiagram
    participant App as Application Thread
    participant SM as ScopedMutex
    participant Mutex as Windows Mutex Handle
    participant CS as CriticalSection
    participant SCS as ScopedCriticalSection
    participant WinCS as Windows CRITICAL_SECTION

    Note over App,WinCS: ScopedMutex Usage (e.g., MilesAudioManager)
    App->>SM: Constructor(HANDLE mutex)
    SM->>Mutex: WaitForSingleObject(mutex, INFINITE)
    Note right of Mutex: Changed from 500ms timeout<br/>to INFINITE wait
    Mutex-->>SM: WAIT_OBJECT_0 (acquired)
    App->>App: Execute protected code
    App->>SM: Destructor
    SM->>Mutex: ReleaseMutex(m_mutex)
    
    Note over App,WinCS: CriticalSection Usage
    App->>CS: Constructor
    CS->>WinCS: InitializeCriticalSection
    App->>SCS: Constructor(CriticalSection*)
    SCS->>CS: enter()
    CS->>WinCS: EnterCriticalSection
    App->>App: Execute protected code
    App->>SCS: Destructor
    SCS->>CS: exit()
    CS->>WinCS: LeaveCriticalSection
    App->>CS: Destructor
    CS->>WinCS: DeleteCriticalSection
Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Unify Unifies code between Generals and Zero Hour ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant