Skip to content

Conversation

@johnmaguire
Copy link
Member

@johnmaguire johnmaguire commented Feb 20, 2021

Fixes #15. When tapping the toggle in rapid succession,
NebulaVpnService.onStartCommand is called twice, in serial. This
method includes logic to show an error to the user if they somehow
attempt to connect to a service while already connected.

However, this method of showing an error message (calling
announceExit) sends a signal to MainActivity telling it the service
has exited, and that it should set the UI state to "Disconnected." It
does not actually disconnect the service at this point, resulting in a
state mismatch in which you cannot actually disconnect the service.

The solution in this commit is to remove this signalling and simply
return out of onStartCommand to avoid processing the start request
twice.

Another potential solution is to call sendSimple(MSG_IS_RUNNING, 1)
before returning, which would ensure that the UI is synchronized with
the service state at this point, however I have no reason to believe it
could get out of sync aside from the processing delay.

@johnmaguire johnmaguire changed the title Fix race when connection toggle is tapped twice Fix state when connection toggle is tapped twice Feb 20, 2021
@johnmaguire johnmaguire closed this May 3, 2021
@johnmaguire johnmaguire reopened this May 3, 2021
Fixes #15. When tapping the toggle in rapid succession,
`NebulaVpnService.onStartCommand` is called twice, in serial.  This
method includes logic to show an error to the user if they somehow
attempt to connect to a service while already connected.

However, this method of showing an error message (calling
`announceExit`) sends a signal to `MainActivity` telling it the service
has exited, and that it should set the UI state to "Disconnected." It
does not actually disconnect the service at this point, resulting in a
state mismatch in which you cannot actually disconnect the service.

The solution in this commit is to remove this signalling and simply
return out of `onStartCommand` to avoid processing the start request
twice.

Another potential solution is to call `sendSimple(MSG_IS_RUNNING, 1)`
before returning, which would ensure that the UI is synchronized with
the service state at this point, however I have no reason to believe it
could get out of sync aside from the processing delay.
@johnmaguire johnmaguire merged commit 1d044a1 into DefinedNet:master May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Race condition with "Connected" toggle on Android

2 participants