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: Handle standby connection issue #92

Merged
merged 1 commit into from
Jan 26, 2023
Merged

Conversation

plougsgaard
Copy link
Collaborator

Fixes #91

(pending more testing)

@plougsgaard
Copy link
Collaborator Author

Further: Some time after a long standby the websocket will receive a close event. It is not possible to distinguish between this and other close events. Will maybe handle it heuristically with timestamps.

@plougsgaard plougsgaard marked this pull request as draft February 7, 2021 17:03
@magnus-madsen
Copy link
Member

Further: Some time after a long standby the websocket will receive a close event. It is not possible to distinguish between this and other close events. Will maybe handle it heuristically with timestamps.

Any reason not just to try to reconnect?

@mR4smussen
Copy link
Contributor

Maybe I’m missing something, but I think e9dcb55 solves the issue. Before these changes the socket connection would disconnect (and not reconnect as intended) shortly after the system had been either on standby or in hibernate mode. After these changes my system seems to reconnected/restart the flix session as soon as it disconnects. This only leaves a short interval where the connection is down (when reconnecting). I don’t know if we had a reason to not merge this or if it was simply never tested?
(The longest standby test I have made is around 1 hour)

To recreate this bug (on my system):

  • Open a flix project (and let flix start/load) - Now the connection should be established like on the image below:
    image
  • Put the computer in standby or hibernate mode (the time doesn’t seem to have an affect for me)
  • Open the computer (and flix project) again - The socket connection should now be in the middle of disconnecting (TIME_WAIT) and soon be disconnected like on the image below:
    image

@magnus-madsen
Copy link
Member

I just think it was never tested. Or maybe it did not work 100%. No idea if @plougsgaard remembers?

@magnus-madsen
Copy link
Member

If you seem satisfied with the fix (both code and testing) then we can merge it in.

@magnus-madsen
Copy link
Member

Maybe you can test, on Linux, by enabling a firewall rule that REJECTs the packages going to VSCode/Flix?

https://serverfault.com/questions/604226/block-an-outbound-destination-ip-to-prevent-connections-waiting-timeout-duration

And then dropping the firewall rule.

@mR4smussen
Copy link
Contributor

If you seem satisfied with the fix (both code and testing) then we can merge it in.

The code looks to me like it should work and I have been testing it for a while now, trying to get the same error and haven’t been able to. I think we should try to merge.

Maybe you can test, on Linux, by enabling a firewall rule that REJECTs the packages going to VSCode/Flix?
https://serverfault.com/questions/604226/block-an-outbound-destination-ip-to-prevent-connections-waiting-timeout-duration
And then dropping the firewall rule.

Maybe I don’t completely understand this. I’m not on a Linux system. If you mean that this is a way to reproduce the bug, I think you could be right. I tried producing the bug in a similar way using windows defender, but I’m not always good friends with that program, so there might also be a way to get it to work on windows (I wasn't able to when i tried)

@magnus-madsen
Copy link
Member

@mR4smussen Does this PR restart the extension, the Flix compiler, or both?

I would say it should not be necessary to restart the Flix compiler (which can be slow). But I am also not sure if that is what is happening.

@mR4smussen
Copy link
Contributor

From what I can see and understand this only restarts the extension. For instance, I can run a flix program as soon as the extension has restarted, without the compiler needing to start up again.

@magnus-madsen
Copy link
Member

Does it display a notification that it has reconnected?

@mR4smussen
Copy link
Contributor

It doesn’t necessarily indicate that the connection was lost and it had to reconnect, it simply shows the “starting flix extension” label and then the “ready” label when the extension is ready.
starting
ready
(no message is displayed in the console)

@magnus-madsen
Copy link
Member

Ok

@magnus-madsen magnus-madsen marked this pull request as ready for review January 26, 2023 19:18
@magnus-madsen magnus-madsen merged commit 90bd98c into master Jan 26, 2023
@magnus-madsen magnus-madsen deleted the fix-wake-on-sleep branch January 26, 2023 19:18
@magnus-madsen
Copy link
Member

@mR4smussen It seems that this PR has broken the action "Restart Flix Compiler". Can you confirm?

@mR4smussen
Copy link
Contributor

That is correct :(. I'm not sure why yet, But I could imagine that since we now start a new compiler when a compiler is closed, a restart could result in two compilers being started at once. In my experience it does work after starting a couple of times, but this is obviously not optimal. I will look for a fix to this tomorrow or in the weekend

@magnus-madsen
Copy link
Member

That is correct :(. I'm not sure why yet, But I could imagine that since we now start a new compiler when a compiler is closed, a restart could result in two compilers being started at once. In my experience it does work after starting a couple of times, but this is obviously not optimal. I will look for a fix to this tomorrow or in the weekend

Sounds good.

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.

Standby still breaks VSCode Connection
3 participants