Add close window support for Windows

[Legend-Master]

Add cleanup support for closing the terminal/shell window on Windows

[codecov-commenter]

Codecov Report

Patch and project coverage have no change.

Comparison is base (6ecc201) 22.62% compared to head (ad0e805) 22.62%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@           Coverage Diff           @@
##              dev    #1111   +/-   ##
=======================================
  Coverage   22.62%   22.62%           
=======================================
  Files          42       42           
  Lines        8601     8601           
=======================================
  Hits         1946     1946           
  Misses       6655     6655           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[hamishcoleman]

I've been trying to understand the logic here.

• What is the problem that is occurring for you without this patch?
• Can we avoid putting a sleep(INFINITE) in there? (can we not just wait for the other threads to exit?)

I'm guessing, but I think what is happening is - when the signal handler returns, windows immediately kills the process and all its threads. So, if we signal the main loop to exit and never return from the signal handler, that gives the main loop time to do any cleanup needed. So, what kills the sleep() thread in the signal handler?

[Legend-Master]

Without it, when you close the terminal/shell window, it won't have the time to clean up which will cause authentication error if you try to reconnect

Blocking this infinitely is fine since when our main thread returns, it will shut down anyway

I'm guessing, but I think what is happening is - when the signal handler returns, windows immediately kills the process and all its threads.

Yes, exactly

I found Go is doing this when I was searching, and there's a comment explained how it works (it also mentioned a library called libuv which is used by Node.js): golang/go#41884 (comment)

[hamishcoleman]

Thanks for expanding on the explanation

[hamishcoleman]

Is it expected that it takes 5 minutes between Ctrl-C and the process actually exiting?

[Legend-Master]

Is it expected that it takes 5 minutes between Ctrl-C and the process actually exiting?

No, I created an issue about this at #1087 2 months ago

And if you mean 5 seconds on hitting the x from the Windows console window, then that's the time when Windows decides to kill the program no matter if it's finished or not

[hamishcoleman]

I wondered if it was related to the sleep(infinite).

I dont run long running tasks in console windows, so I would never close those windows from hitting the x.

These select timeouts make sense on everything other than windows. Lets keep that other ticket open and I will see if I can look at it some time.

[Legend-Master]

I wondered if it was related to the sleep(infinite).

I don't think it runs on ctrl c

These select timeouts make sense on everything other than windows.

I will take a look on select, if I remembered correctly the shutdown is all good when I ran the supernode on linux, but not on windows

[hamishcoleman]

The select() timeout should be 10 seconds, but I am seeing anything from 3 minutes to 9 minutes!

The messages clearly show that it is going through the term_handler() codepath, but I dont have a debugger installed on my windows test VMs, so I have not traced the exact place it is hanging - so it could be anywhere.

I don't think I have seen these kind of delays on a linux systems - I would remember.