Fix edge slow shutdown on Windows

[Legend-Master]

Fix #1087 partially

[codecov-commenter]

Codecov Report

Attention: 79 lines in your changes are missing coverage. Please review.

Comparison is base (fff50a7) 21.58% compared to head (1ca14fc) 21.47%.

Files	Patch %	Lines
src/edge_utils.c	0.00%	79 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1121      +/-   ##
==========================================
- Coverage   21.58%   21.47%   -0.12%     
==========================================
  Files          46       46              
  Lines        8991     9038      +47     
==========================================
  Hits         1941     1941              
- Misses       7050     7097      +47     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[hamishcoleman]

This is quite an invasive change. I would like to see less ifdefs and specific platform code in the core - not more.

Perhaps we can consider other options?

[Legend-Master]

This is quite an invasive change. I would like to see less ifdefs and specific platform code in the core - not more.

I would like too, but I don't think there's anyway to interrupt select

Perhaps we can consider other options?

I would say just make pulling timeout shorter on Windows, like 1-3 seconds (I'm currently doing this for my own use)

[hamishcoleman]

I see you have changed the winsock include - is there a reason for that? is there any side effects? should we consider doing that as a separate update to the timeout PR?

[Legend-Master]

I see you have changed the winsock include - is there a reason for that? is there any side effects? should we consider doing that as a separate update to the timeout PR?

WSAWaitForMultipleEvents only exits in winsock2, and I heard mixing them is problematic on https://stackoverflow.com/a/9168850

winsock2 should be fully compatible in the current code base, there're only a few thing like WSACancelBlockingCall were removed (I tried this function to cancel select but can't make it work even with winsock.h)

If we choose to use the reduced timeout method, then this change is not needed

[hamishcoleman]

Yeah, I've read that SO article a while ago - but I'm not sure why this codebase has the older header.

Unfortunately the windows source code environment seems to be quite hostile to the usual programming techniques that people have been using for more decades than windows has existed :-( -- if the new header is a 100% replacement for the old header (excepting functions you have chosen to drop support for), then just replace it, dont hand the future a maintenance problem!

[Legend-Master]

So what do you think, reduce timeout or WSAWaitForMultipleEvents or anything else?

I think the reduced timeout is fine after using it for a few months

If we choose the timeout one, then I think we can make another PR to unify winsock.h to winsock2.h (or reverse)

[hamishcoleman]

I believe all these slow windows shutdown concerns are fixed in the recently released fork n3n.
So, if you could to try that out and and raise a ticket there if it is still happening, we would be happy to investigate it further as part of that new project.

[Legend-Master]

Just tried it, worked well with connect and then ctrl-c

Closing this pull request then