refactor(state): 将状态机实现为完全异步操作并改进线程安全机制#28
Merged
Conversation
- 添加 SemaphoreSlim 锁确保状态转换的线程安全性 - 将所有同步方法重构为异步方法并移除旧的同步实现 - 使用异步锁替代传统的 lock 机制提升并发性能 - 优化状态历史记录的处理时机和逻辑 - 移除过时的同步状态转换内部方法 - 统一异常处理和资源释放机制
Reviewer's GuideRefactors the state machine to use fully asynchronous state transitions with a dedicated SemaphoreSlim-based transition lock, removes legacy synchronous transition paths, and adjusts history/notification ordering to be consistent with async execution and thread safety. Sequence diagram for asynchronous state transition with SemaphoreSlim locksequenceDiagram
actor Client
participant StateMachineSystem
participant TransitionLock as SemaphoreSlim_transitionLock
participant OldState as Old_state
participant NextState as Next_state
Client->>StateMachineSystem: ChangeToAsync~T~()
StateMachineSystem->>TransitionLock: WaitAsync()
TransitionLock-->>StateMachineSystem: lock_acquired
StateMachineSystem->>StateMachineSystem: Resolve target state T
StateMachineSystem->>StateMachineSystem: Get Current snapshot
alt has current state
StateMachineSystem->>OldState: CanTransitionToAsync(Old_state, Next_state)
alt transition_rejected
StateMachineSystem->>StateMachineSystem: OnTransitionRejectedAsync(Old_state, Next_state)
StateMachineSystem-->>Client: false
StateMachineSystem->>TransitionLock: Release()
else transition_allowed
StateMachineSystem->>StateMachineSystem: ChangeInternalAsync(Next_state)
StateMachineSystem->>StateMachineSystem: AddToHistory(Old_state)
StateMachineSystem->>OldState: ExecuteExitAsync(Old_state, Next_state)
StateMachineSystem->>NextState: ExecuteEnterAsync(Next_state, Old_state)
StateMachineSystem->>StateMachineSystem: OnStateChangedAsync(Old_state, Next_state)
StateMachineSystem-->>Client: true
StateMachineSystem->>TransitionLock: Release()
end
else no current state
StateMachineSystem->>StateMachineSystem: ChangeInternalAsync(Next_state)
StateMachineSystem->>StateMachineSystem: AddToHistory(null)
StateMachineSystem->>NextState: ExecuteEnterAsync(Next_state, null)
StateMachineSystem->>StateMachineSystem: OnStateChangedAsync(null, Next_state)
StateMachineSystem-->>Client: true
StateMachineSystem->>TransitionLock: Release()
end
Class diagram for the refactored asynchronous StateMachineclassDiagram
class IState {
}
class IStateMachine {
}
class StateMachine {
-object _lock
-HashSet~IState~ _registeredStates
-Stack~IState~ _stateHistory
-SemaphoreSlim _transitionLock
+IState Current
+Dictionary~Type, IState~ States
+StateMachine(maxHistorySize int)
+IStateMachine Register(state IState)
+IStateMachine Unregister~T~()
+Task~IStateMachine~ UnregisterAsync~T~()
+bool CanChangeTo~T~()
+Task~bool~ CanChangeToAsync~T~()
+bool ChangeTo~T~()
+Task~bool~ ChangeToAsync~T~()
+IReadOnlyList~IState~ GetStateHistory()
+bool GoBack()
+Task~bool~ GoBackAsync()
-IState PrepareUnregister~T~(isCurrentState bool)
-void CompleteUnregister(stateToUnregister IState)
-IState FindValidPreviousState()
-void AddToHistory(state IState)
-Task ChangeInternalWithoutHistoryAsync(next IState)
-Task ChangeInternalAsync(next IState)
-Task~bool~ CanTransitionToAsync(current IState, target IState)
-Task ExecuteExitAsync(old IState, next IState)
-Task ExecuteEnterAsync(next IState, previous IState)
-Task OnStateChangingAsync(old IState, next IState)
-Task OnStateChangedAsync(old IState, next IState)
-Task OnTransitionRejectedAsync(current IState, target IState)
}
class StateMachineSystem {
+void Destroy()
+Task ChangeInternalWithoutHistoryAsync(next IState)
+Task ChangeInternalAsync(next IState)
}
IStateMachine <|.. StateMachine
IState <|.. StateMachine
StateMachineSystem --|> StateMachine
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The synchronous wrappers (
Unregister,CanChangeTo,ChangeTo,GoBack) now just call the async versions viaGetAwaiter().GetResult(), which can cause deadlocks and defeats the goal of a fully async API; consider either removing the sync API or clearly isolating/blocking it outside of the async locking path. ChangeToAsynccombines_transitionLock.WaitAsync()with an innerlock (_lock)to readStates/Current; consolidating to a single synchronization primitive (e.g., only_transitionLock) would simplify the concurrency model and avoid potential deadlock ordering issues.- Since
_locknow appears to be used only inChangeToAsync, consider either removing it entirely in favor of the new async semaphore or clearly separating responsibilities between the two locks to avoid confusion for future maintainers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The synchronous wrappers (`Unregister`, `CanChangeTo`, `ChangeTo`, `GoBack`) now just call the async versions via `GetAwaiter().GetResult()`, which can cause deadlocks and defeats the goal of a fully async API; consider either removing the sync API or clearly isolating/blocking it outside of the async locking path.
- `ChangeToAsync` combines `_transitionLock.WaitAsync()` with an inner `lock (_lock)` to read `States`/`Current`; consolidating to a single synchronization primitive (e.g., only `_transitionLock`) would simplify the concurrency model and avoid potential deadlock ordering issues.
- Since `_lock` now appears to be used only in `ChangeToAsync`, consider either removing it entirely in favor of the new async semaphore or clearly separating responsibilities between the two locks to avoid confusion for future maintainers.
## Individual Comments
### Comment 1
<location> `GFramework.Core/state/StateMachine.cs:48-51` </location>
<code_context>
-
- CompleteUnregister(stateToUnregister);
- return this;
+ return UnregisterAsync<T>().GetAwaiter().GetResult();
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Using GetAwaiter().GetResult() on async methods can introduce deadlock risk and subtle context issues.
All synchronous APIs (`Unregister`, `CanChangeTo`, `ChangeTo`, `GoBack`) now block on their async counterparts. Because those async flows await `SemaphoreSlim.WaitAsync` and invoke user-provided callbacks (`ExecuteEnterAsync`, `ExecuteExitAsync`, `CanTransitionToAsync`, etc.), any captured synchronization context (UI, ASP.NET classic, context-bound schedulers) can cause deadlocks for sync callers.
Consider either (1) keeping a truly synchronous path that shares a non-async core; (2) explicitly limiting usage to context-free/thread-pool scenarios; or (3) moving to async-only transition APIs and deprecating the sync ones. The current design makes it easy for callers to introduce deadlocks in common hosting environments.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- 移除 IStateMachine 接口中 Unregister<T>() 方法的同步定义 - 移除 StateMachine 类中 Unregister<T>() 方法的同步实现 - 移除 IStateMachine 接口中 CanChangeTo<T>() 方法的同步定义 - 移除 StateMachine 类中 CanChangeTo<T>() 方法的同步实现 - 移除 IStateMachine 接口中 ChangeTo<T>() 方法的同步定义 - 移除 StateMachine 类中 ChangeTo<T>() 方法的同步实现 - 移除 IStateMachine 接口中 GoBack() 方法的同步定义 - 移除 StateMachine 类中 GoBack() 方法的同步实现
- 删除了 StateMachineSystemTests.cs 中关于 ChangeTo 方法的基本功能测试 - 删除了 StateMachineTests.cs 中关于状态切换、注册注销等基础功能的测试用例 - 保留了异步操作相关的测试方法以简化测试套件 - 减少了测试文件的代码量并提高维护效率
This was referenced Mar 15, 2026
This was referenced May 10, 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.
Summary by Sourcery
Refactor the state machine to use fully asynchronous, thread-safe state transitions and simplify the public API by routing synchronous calls through async implementations.
Enhancements: