Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix VPN client duplicates #642

Merged
merged 1 commit into from
Dec 17, 2020
Merged

Conversation

Darkren
Copy link
Contributor

@Darkren Darkren commented Dec 15, 2020

Did you run make format && make check?
Yes
Fixes #641 #636

Changes:

  • Now VPN client exits gracefully during dialing. Previously dialing couldn't be interrupted. So, when we changed anything or tried to stop the app, the request just hung waiting for the process to stop. And this waiting eventually gets interrupted with an error. In this case we just decouple process from the visor due to the internal architecture, therefore allowing app duplicates. However, this shouldn't be a problem for a well-written (in terms of interruption) apps

How to test this PR:
Repeat steps from the original ticket. Step 3 should complete and app should be restarted. Also VPN client should be closed on visor shut down without any duplicates. Also, previously issue could be reproduced not by changing the app arg, but just stopping it and refreshing page afterwards (when stop hung). This should work too, app should be stopped ok during dialing

@Darkren Darkren changed the base branch from master to develop December 15, 2020 15:20
Copy link
Member

@jdknives jdknives left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works. Nice job.

@i-hate-nicknames
Copy link
Contributor

Great job, works for me too.

@jdknives jdknives merged commit 5b0f027 into skycoin:develop Dec 17, 2020
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.

App startup/shutdown issues
3 participants