search

nguyenquyhy / Flight-Recorder

Record and replay flights in Microsoft Flight Simulator
https://flightsim.to/file/8163/flight-recorder
GNU General Public License v3.0
207 stars 21 forks source link

Fix race condition when simconnect is disconnected during saving #84

Closed nguyenquyhy closed 3 years ago

nguyenquyhy commented 3 years ago

Problem Description

2021-05-18 01:52:58.347 -04:00 [INF] Triggered event "Disconnect" from state "SavingIdle" to "SavingDisconnected"
2021-05-18 01:53:02.990 -04:00 [INF] Triggered event "Save" due to "Save" from state "SavingIdle" to "IdleSaved"

In this case, final state should be IdleDisconnected instead of IdleSaved.

This gets even more tricky when reverting happen because reverting is current handled by remembering the initial state, which can change due to the event sequences.

Right now: IdleUnsaved --Save--> SavingIdle --Disconnect--> SavingDisconnected --Cancel(Revert)--> IdleUnsaved DisconnectedEmpty --Load--> LoadingDisconnected --Connect--> LoadingIdle --Cancel(Revert)--> DisconnectedEmpty

Sample logs:

Triggered event "RequestLoading" due to "Load" from state "DisconnectedEmpty" to "LoadingDisconnected"
Triggered event "Connect" from state "LoadingDisconnected" to "LoadingIdle"
Triggered event "Load" due to "Load" from state "LoadingDisconnected" to null
# Transition from "LoadingDisconnected" by "Load" was cancelled! Revert back to orignal state "DisconnectedEmpty". #

Expected: IdleUnsaved --Save--> SavingIdle --Disconnect--> SavingDisconnected --Cancel(Revert)--> DisconnectedUnsaved

Root Causes

  1. Long running processing is included in the transition, which means there is a sequence of logic that can cause race condition easily: get the current state -> identify the transition -> long processing -> change to end state.
    • Saving/Loading
    • Confirmation dialog
  2. Reverting (Cancelling) is done by remembering the initial state and reset to that state if the transition is cancelled. Another separate transition that happen between the initial transition and the cancellation will invalidate the cached initial state.
nguyenquyhy commented 3 years ago

Released in v0.17