Skip to content

Fix stale remote server picker responses overwriting newer ones - #367

Open
coderandhiker wants to merge 1 commit into
satisfactorymodding:masterfrom
coderandhiker:fix/remote-server-picker-race
Open

coderandhiker wants to merge 1 commit into
satisfactorymodding:masterfrom
coderandhiker:fix/remote-server-picker-race

Conversation

@coderandhiker

Copy link
Copy Markdown

restartPicker is debounced, but a StartPicker call already in flight is not cancelled when the base path changes again. Typing the host before the port starts a picker for the default port; if that attempt is still failing when the real port is entered, its error lands after the newer picker succeeded and replaces the working state with a stale "failed to connect" error.

Capture the base path before awaiting, and discard the response (releasing the picker it created) if the base path changed or the picker was disabled in the meantime, the same way checkValid already discards stale TryPick responses.

image

In the error message shown, my IP followed by port 22 was shown, I had typed a different port number.

I encountered this issue while working on satisfactorymodding/ficsit-cli#86, which refs #304

restartPicker is debounced, but a StartPicker call already in flight is not
cancelled when the base path changes again. Typing the host before the port
starts a picker for the default port; if that attempt is still failing when
the real port is entered, its error lands after the newer picker succeeded
and replaces the working state with a stale "failed to connect" error. Had
the stale attempt succeeded instead, it would have replaced pickerId with a
picker for the wrong url and leaked the newer one.

Capture the base path before awaiting, and discard the response (releasing
the picker it created) if the base path changed or the picker was disabled
in the meantime, the same way checkValid already discards stale TryPick
responses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🆕 New

Development

Successfully merging this pull request may close these issues.

1 participant