win,tcp: make uv_close work more like unix #3036

vtjnash · 2020-11-04T20:38:29Z

This is an attempt to fix some TCP resource management issues on Windows and avoid a bug where it sends a RST packet that is unwarranted. The Win32 sockets library appear to have a bug where it sends an RST packet if there is an outstanding overlapped WSARecv call, even if no data is received. This would be akin to epoll/kevent sending RST if you close the socket while it was still registered—and so our fix is similar to the existing unix code: calling CancelIOEx on Windows is akin to calling uv__io_stop on unix. Thus we can avoid the observed problem by being certain to explicitly cancel our read request first, and not just buried in unlikely conditional cases. If this approach fails for people (this capability was only added in Vista), we could also switch to using select on a background thread to pump these events, but it would be nice to avoid doing that extra coding work.

This also removes some conditional cleanup code, since we might as well clean it up eagerly (like unix). Otherwise, it looks to me like these might cause the accept callbacks to be run after the endgame had freed the memory for them (or not at all)?

The comment here seems to have gotten slightly mixed up over time between send and recv buffers and the expected vs. incorrect failures this code encountered. The default behavior on calling closesocket is already to do a graceful shutdown (see https://docs.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-closesocket
with default l_onoff=zero) if it is the last open handle. So the expected behavior if there are pending reads in flight is to send an RST packet, notifying the client that the server connection was destroyed before acknowledging the EOF. But instead we also observe an RST packet being sent if even though there are no messages in flight.

The only test that I noted already exercises this failure case (after removing the conditionals, so that we aren't injecting a uv_shutdown call anymore and the bug is detectable) is ipc_tcp_connection.

This PR is passing on CI ✅: https://ci.nodejs.org/job/libuv-test-commit-windows-cmake/273/

vtjnash · 2020-11-05T15:04:56Z

This reliably fails one benchmark test on libuv-in-node CI, though the error reporting seems quite useless to tell more without more involved investigations:
https://ci.nodejs.org/job/node-test-binary-windows-native-suites/5868/nodes=win10-vcbt2015-COMPILED_BY-vs2019/testReport/junit/(root)/test/benchmark_test_benchmark_tls/

node:events:304
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at TCP.onStreamRead (node:internal/stream_base_commons:213:20)
Emitted 'error' event on Socket instance at:
    at Socket.onerror (node:internal/streams/readable:756:14)
    at Socket.emit (node:events:327:20)
    at emitErrorNT (node:internal/streams/destroy:188:8)
    at emitErrorCloseNT (node:internal/streams/destroy:153:3)
    at processTicksAndRejections (node:internal/process/task_queues:80:21) {
  errno: -4077,
  code: 'ECONNRESET',
  syscall: 'read'
}

This is an attempt to fix some resource management issues on Windows. Win32 sockets have an issue where it sends an RST packet if there is an outstanding overlapped calls. We can avoid that by being certain to explicitly cancel our read and write requests first. This also removes some conditional cleanup code, since we might as well clean it up eagerly (like unix). Otherwise, it looks to me like these might cause the accept callbacks to be run after the endgame had freed the memory for them. The comment here seems mixed up between send and recv buffers. The default behavior on calling `closesocket` is already to do a graceful shutdown (see https://docs.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-closesocket with default l_onoff=zero) if it is the last open handle. The expected behavior if there are pending reads in flight is to send an RST packet, notifying the client that the server connection was destroyed before acknowledging the EOF. Refs: libuv#3035 Refs: nodejs/node#35946 Refs: nodejs/node#35904 Fixes: libuv#3034 PR-URL: libuv#3036

The prior attempt was good for reads, but ignored writes. We need to retire those first too, before calling closesocket.

The prior attempt was great for reads and writes, but not for cancelation. The expected behavior is for writes to be canceled (not finished) on uv_close, while in attempt 2 we were letting them finish. We need to explicitly notify Win32 that it is okay to cancel these writes (so it doesn't also generate an RST packet on the wire).

vtjnash · 2020-11-10T17:31:16Z

First attempt was showing good improvement, but I'd neglected to deal with writes also, which was the cause of that nodejs test now reliably failing. The followup commits should now handle those also. (@mmomtchev)

CI ✅: https://ci.nodejs.org/job/libuv-test-commit-windows-cmake/279/
Node: https://ci.nodejs.org/view/libuv/job/libuv-in-node/170/

mmomtchev · 2020-11-12T14:57:44Z

@vtjnash I manually applied this as a patch to the 1.40 in Node-master and I still get a RST on the loopback interface on test/parallel/test-https-truncate.js

vtjnash · 2020-11-12T17:40:15Z

I am looking at that test now, and am a bit confused. It appears to me that, per the TCP spec, the client should be sending an RST message to the server, so the server learns that the client no longer cares about the data remaining in its send buffer. Sometimes the message may be dropped in transit though as an optimization, if it learns that the server already doesn't care. Thus, there appears to have been a partial workaround / optimization implemented in the PR that added that test, but no actual handling for the condition in that PR (so may be worth removing that patch, to surface the error itself in regular testing). However, http does seem to have an error handler which should do the right thing(tm) and quietly tear down the socket: https://github.com/nodejs/node/blob/e6e64f7a21f5570b5da780c8162fe10dc7c62be9/lib/_http_server.js#L654-L657

But you're sure it's an RST from server -> client that you're now seeing (per comment in your original issue / PR)?

vtjnash · 2020-11-12T19:26:37Z

Okay, yes, I see that sequence in wireshark. Googling it seems to suggest that the RST may be due to a TLS protocol violation, and is warning of some sort of vulnerability to https://en.wikipedia.org/wiki/Transport_Layer_Security#Truncation_attack (I'm unclear on the exact details of that hack). This is clarified by reading the standard https://tools.ietf.org/html/rfc5246#section-7.2.1 however, which notes that attack, and describes the behavior of nodejs as being valid.

Here's a sample record of running that test on windows, with TLS decryption (--tls-keylog), in Wireshark, with these works in progress applied:

50294 is the server, 50295 the client here
close notify 12, and subsequent FIN 14, is from server
ACK 15, and subsequent close notify 16, is from the client
RST 17 from the server then informs the client that the server was already disconnected (oops)

So what I think appears to be the case is that nodejs is failing to follow this requirement of the standard:
https://tools.ietf.org/html/rfc5246#section-7.2.1

The other party MUST respond with a close_notify
alert of its own and close down the connection immediately

So by my reading, the standard here requires that this read error be ignored for http (as it's after the close_notify) event, and that nodejs https should have already initiate destroy on the socket itself by this time.

Other protocols (not https) on top of TLS are allowed to continue receiving data after the close_notify, but in that case, the server would be in violation to have destroyed the socket already.

vtjnash · 2020-11-12T21:12:47Z

To zero'th order, it looks like nodejs deletes the socket 'end' handler after the headers are done. So we can fix your observed bug by putting back a method that just matches does what the standard says here and destroys the socket:

diff --git a/lib/_http_client.js b/lib/_http_client.js
index f2c0dcc98c..4898c06182 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -567,6 +567,11 @@ function socketOnData(d) {
     socket.removeListener('end', socketOnEnd);
     socket.removeListener('drain', ondrain);
     freeParser(parser, req, socket);
+    socket.on('end', () => {
+      // ignore any data (or errors) that arrive over TLS after the initiation
+      // of the close_notify handshake for the http protocol
+      socket.destroy();
+    });
   }
 }

This is isn't necessary here for plain http, since the only way you get an EOF notification from a basic socket is for the stream to be done emitting read events (the read end is already destroyed). However, for TLS, the standard allows for data to continue to arrive after EOF, so the read end is not yet destroyed at that time. If the protocol (in this case http) doesn't permit data (or errors) to arrive after this event, it thus needs to explicitly call destroy on the stream.

Perhaps this could even be added to the meaning of autoDestroy (handled in onReadableStreamEnd)? I don't know the appropriate place for this to be handled, since it's the responsibility of the http client to indicate to TLS that it doesn't expect any more data (and wants to immediately cancel any writes)

This patch isn't quite complete however, since it would get added too many times with KeepAlive. Possibly, like the TODO several lines later for handling errors for onSocket, we want this only to be active when it's not being actively used for something else.

mmomtchev · 2020-11-12T23:06:41Z

@vtjnash I didn't read the TLS truncation in detail, but it can't be this for sure - the TLS server, which is running Node, has no RST capability - this is still not implemented. Until the uv_tcp_reset is implemented in Node, a RST from a server running Node can come only from the OS kernel.
I will look into the HTTP client fix, but I think that the final data packet comes from the OS kernel too - its being sent in response to the FIN of the server. The only way to really fix this on the client would be to change the closing to simultaneous closing (which is what we do in almost every other case) - but this would still be a partial solution because the client could be running something else than Node. For me the only viable long-term solution is to support active close. And on the short term - it is probably best to continue ignoring the RST - no matter how awful it is.

vtjnash · 2020-11-13T02:17:27Z

There's many ways that an application can trigger an RST, the uv_tcp_close_reset is just a particularly simple way, while other ways can be more complex and require involvement from the other side. In this case, it's being triggered explicitly by the nodejs client/server handshake that should terminate an https connection, per the TLS standard. The failure here is that the http application is supposed to mask/ignore this particular RST packet, as it arrives after the http protocol has declared that the TLS socket should be destroyed. My patch above fixes that http protocol violation in nodejs, as mandated by rfc5246#section-7.2.1 above. I think that's what you mean by "simultaneous closing" behavior?

I'm not sure what you mean by "active close". Do you mean that the server should wait to destroy the socket until after the close_notify handshake is finished? If so, that's a valid behavior, but isn't mandated (or particularly recommended), so other servers won't be expected to follow that rule, and thus the client would still needs the real patch.

stale · 2020-12-04T21:07:20Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

RFC 5246 section-7.2.1 requires that the implementation must immediately stop reading from the stream, as it is no longer TLS-encrypted. The underlying stream is permitted to still pump events (and errors) to other users, but those are now unencrypted, so we should not process them here. But therefore, we do not want to stop the underlying stream, as there could be another user of it, but we do need to remove ourselves as a listener. Per TLS v1.2, we should have also destroy the TLS state entirely here (including the writing side), but this was revised in TLS v1.3 to permit the stream to continue to flush output. There appears to be some inconsistencies in the way nodejs handles ownership of the underlying stream, with `TLS.close()` on the write side also calling shutdown on the underlying stream (thus assuming other users of the underlying stream are not permitted), while receiving EOF on the read side leaves the underlying channel open. These inconsistencies are left for a later person to resolve, if the extra functionality is needed (as described in nodejs#35904). The current goal here is to the fix the occasional CI exceptions depending on the timing of these kernel messages through the TCP stack. Refs: libuv/libuv#3036 Refs: nodejs#35904 Closes: nodejs#35946 Co-authored-by: Momtchil Momtchev <momtchil@momtchev.com>

vtjnash · 2021-05-10T23:42:16Z

Since I submitted the fix to nodejs for their missing error check 6 months ago (nodejs/node#36111), I'd like to merge this now. Anyone willing to review?

vtjnash · 2021-05-20T14:50:17Z

bump. This fixes some connection management issues on Windows, and aligns the code with the correct behaviors that we are doing unix. Anyone willing to approve this? Thanks!

vtjnash · 2021-05-24T20:28:28Z

bump

vtjnash · 2021-05-27T14:46:40Z

bump???

vtjnash · 2021-05-31T15:34:30Z

daily bump?

vtjnash · 2021-06-01T20:57:17Z

daily bump?

vtjnash · 2021-06-02T17:03:49Z

daily bump? I've got to stay ahead of stalebot, since it seems pretty eager to close issues that were being actively worked on but are lacking reviewer approvals, like this one.

vtjnash · 2021-06-03T14:31:11Z

daily bump? I'm content with just a SGTM or rubber stamp too, since if there are any additional issues discovered later, I will be around to work on them. Failing that, I suppose I'll merge next week, as I'd like to make a new release.

vtjnash · 2021-06-05T16:11:59Z

daily bump?

vtjnash · 2021-06-08T14:11:04Z

daily bump? I'll plan to merge in a couple days, then start the release process

santigimeno

SGTM

santigimeno · 2021-06-08T16:08:14Z

src/win/tcp.c

+  if (socket != tcp->socket) {
+    if (reading)
+      CancelIoEx((HANDLE) socket, &tcp->read_req.u.io.overlapped);
+    if (writing)
+      CancelIo((HANDLE) socket);
  }


Sorry for my ignorance but, when can this happen?

socket is the underlying (real) resource while tcp->socket is the user handle (potentially an LSP). It is a deprecated feature in the Window's kernel that these can be different (since it is about as much of an awkward design as it sounds), but still may show up in some ancient firewall products. Those ancient products often may not implement the Windows IO stack correctly either, so we propagate the CancelIO explicitly, rather than relying on them to know about this "modern" functionality.

santigimeno · 2021-06-08T16:09:48Z

test/test-ipc.c

@@ -203,7 +203,7 @@ static void on_read(uv_stream_t* handle,
    /* Make sure that the expected data is correctly multiplexed. */
    ASSERT_MEM_EQ("hello\n", buf->base, nread);

-    outbuf = uv_buf_init("world\n", 6);
+    outbuf = uv_buf_init("foobar\n", 7);


Why this change?

I changed this because I wanted the read and writes to be different lengths ("hello" and "world" were previously the same number of bytes)

RFC 5246 section-7.2.1 requires that the implementation must immediately stop reading from the stream, as it is no longer TLS-encrypted. The underlying stream is permitted to still pump events (and errors) to other users, but those are now unencrypted, so we should not process them here. But therefore, we do not want to stop the underlying stream, as there could be another user of it, but we do need to remove ourselves as a listener. Per TLS v1.2, we should have also destroy the TLS state entirely here (including the writing side), but this was revised in TLS v1.3 to permit the stream to continue to flush output. There appears to be some inconsistencies in the way nodejs handles ownership of the underlying stream, with `TLS.close()` on the write side also calling shutdown on the underlying stream (thus assuming other users of the underlying stream are not permitted), while receiving EOF on the read side leaves the underlying channel open. These inconsistencies are left for a later person to resolve, if the extra functionality is needed (as described in nodejs#35904). The current goal here is to the fix the occasional CI exceptions depending on the timing of these kernel messages through the TCP stack. Refs: libuv/libuv#3036 Refs: nodejs#35904 Closes: nodejs#35946 Co-authored-by: Momtchil Momtchev <momtchil@momtchev.com>

RFC 5246 section-7.2.1 requires that the implementation must immediately stop reading from the stream, as it is no longer TLS-encrypted. The underlying stream is permitted to still pump events (and errors) to other users, but those are now unencrypted, so we should not process them here. But therefore, we do not want to stop the underlying stream, as there could be another user of it, but we do need to remove ourselves as a listener. Per TLS v1.2, we should have also destroy the TLS state entirely here (including the writing side), but this was revised in TLS v1.3 to permit the stream to continue to flush output. There appears to be some inconsistencies in the way nodejs handles ownership of the underlying stream, with `TLS.close()` on the write side also calling shutdown on the underlying stream (thus assuming other users of the underlying stream are not permitted), while receiving EOF on the read side leaves the underlying channel open. These inconsistencies are left for a later person to resolve, if the extra functionality is needed (as described in #35904). The current goal here is to the fix the occasional CI exceptions depending on the timing of these kernel messages through the TCP stack. PR-URL: #36111 Fixes: #35946 Refs: libuv/libuv#3036 Refs: #35904 Co-authored-by: Momtchil Momtchev <momtchil@momtchev.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

RFC 5246 section-7.2.1 requires that the implementation must immediately stop reading from the stream, as it is no longer TLS-encrypted. The underlying stream is permitted to still pump events (and errors) to other users, but those are now unencrypted, so we should not process them here. But therefore, we do not want to stop the underlying stream, as there could be another user of it, but we do need to remove ourselves as a listener. Per TLS v1.2, we should have also destroy the TLS state entirely here (including the writing side), but this was revised in TLS v1.3 to permit the stream to continue to flush output. There appears to be some inconsistencies in the way nodejs handles ownership of the underlying stream, with `TLS.close()` on the write side also calling shutdown on the underlying stream (thus assuming other users of the underlying stream are not permitted), while receiving EOF on the read side leaves the underlying channel open. These inconsistencies are left for a later person to resolve, if the extra functionality is needed (as described in nodejs#35904). The current goal here is to the fix the occasional CI exceptions depending on the timing of these kernel messages through the TCP stack. PR-URL: nodejs#36111 Fixes: nodejs#35946 Refs: libuv/libuv#3036 Refs: nodejs#35904 Co-authored-by: Momtchil Momtchev <momtchil@momtchev.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

RFC 5246 section-7.2.1 requires that the implementation must immediately stop reading from the stream, as it is no longer TLS-encrypted. The underlying stream is permitted to still pump events (and errors) to other users, but those are now unencrypted, so we should not process them here. But therefore, we do not want to stop the underlying stream, as there could be another user of it, but we do need to remove ourselves as a listener. Per TLS v1.2, we should have also destroy the TLS state entirely here (including the writing side), but this was revised in TLS v1.3 to permit the stream to continue to flush output. There appears to be some inconsistencies in the way nodejs handles ownership of the underlying stream, with `TLS.close()` on the write side also calling shutdown on the underlying stream (thus assuming other users of the underlying stream are not permitted), while receiving EOF on the read side leaves the underlying channel open. These inconsistencies are left for a later person to resolve, if the extra functionality is needed (as described in #35904). The current goal here is to the fix the occasional CI exceptions depending on the timing of these kernel messages through the TCP stack. PR-URL: #36111 Fixes: #35946 Refs: libuv/libuv#3036 Refs: #35904 Co-authored-by: Momtchil Momtchev <momtchil@momtchev.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

twose · 2022-03-30T19:04:49Z

99eb736#diff-3f206469017f9748c5a5247f9ff4e99ba43f95a8b0e7ee4a946dbdcc2038a100R1487

Hi @vtjnash !
A socket read timeout test failed in my project, is the problem related to this change?
TCP client uses uv_read_start() to wait for data and it timedout after 1ms then call uv_read_stop() and uv_close(), in early versions, server-side would get return value 0 from recv() (means that the peer client closed the connection normally), but now it got -1 with ECONNREST (RST packet?).
But the same test passed on UNIX-like systems, just broken on Windows.

vtjnash · 2022-03-31T18:10:32Z

Yes, Windows may be violating the TCP spec here, but that spec doesn't say much about the intended behavior of threads. As you see in that part of the diff, we try to instruct Windows not to send the RST packet, but it looks like we don't execute that logic after uv_read_stop, so we end up sending the unwanted RST packet. The problem appears to be that we don't have a uv__tcp_read_stop function, so we never cancel the old req, even when we might later start a new one with the same overlapped handle.

Actually, that is probably a fairly serious existing bug. It appears we may need to more closely track the lifetime of that pointer that we have passed into the pointer, and make sure we are not going to finalize the uv_close callback until we have confirmation the kernel has stopped tracking that pointer. We have called closesocket, so those notification will happen soon, but we must wait for the overlapped callback before we run the endgame and free that memory.

vtjnash · 2022-03-31T18:11:18Z

Would you be interested in trying to make a PR to manage those state transitions more accurately?

twose · 2022-03-31T19:41:13Z

@vtjnash
I'm very interested in this, but I can not make any guarantees.
If you're willing to wait, I'll try to fix it this week. It would be better if you could talk more about these before I try, like If it were you, how would you consider fixing it 😋 , or, how do you expect me to do?

vtjnash · 2022-03-31T20:37:36Z

The main change is to use UV_HANDLE_READ_PENDING instead of UV_HANDLE_READING in a couple places during closing, to make sure we are considering the state of the overlapped buffer, instead of the state of the libuv callbacks there. Upon closer inspection, we handle that correctly already in most circumstances, but I changed it to the wrong flag here and we don't properly wait for that notification to arrive that the cancellation is finished before the endgame.

This is an attempt to fix some resource management issues on Windows. Win32 sockets have an issue where it sends an RST packet if there is an outstanding overlapped calls. We can avoid that by being certain to explicitly cancel our read and write requests first. This also removes some conditional cleanup code, since we might as well clean it up eagerly (like unix). Otherwise, it looks to me like these might cause the accept callbacks to be run after the endgame had freed the memory for them. The comment here seems mixed up between send and recv buffers. The default behavior on calling `closesocket` is already to do a graceful shutdown (see https://docs.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-closesocket with default l_onoff=zero) if it is the last open handle. The expected behavior if there are pending reads in flight is to send an RST packet, notifying the client that the server connection was destroyed before acknowledging the EOF. Additionally, we need to cancel writes explicitly: we need to notify Win32 that it is okay to cancel these writes (so it doesn't also generate an RST packet on the wire). Refs: libuv#3035 Refs: nodejs/node#35946 Refs: nodejs/node#35904 Fixes: libuv#3034 PR-URL: libuv#3036 Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>

vtjnash mentioned this pull request Nov 4, 2020

Closing a socket with a pending read makes Windows send a TCP RST packet #3034

Closed

vtjnash added 3 commits November 10, 2020 11:58

fixup! attempt 2

55dcd3c

The prior attempt was good for reads, but ignored writes. We need to retire those first too, before calling closesocket.

vtjnash force-pushed the jn/tcp-close-win-rst branch from 38e2978 to ab3e742 Compare November 10, 2020 17:08

This was referenced Nov 13, 2020

TLS: improve compliance with shutdown standard, remove hacks nodejs/node#36111

Closed

Using a Foreign Event loop with a client websocket (outbound) connection warmcat/libwebsockets#2108

Closed

stale bot added the stale label Dec 4, 2020

vtjnash mentioned this pull request Jan 6, 2021

Release proposal: v1.41.0 #3086

Closed

tniessen mentioned this pull request Mar 25, 2021

Node.js Technical Steering Committee (TSC) Meeting 2021-03-25 nodejs/TSC#999

Closed

stale bot removed the stale label May 10, 2021

vtjnash added the pending-review label May 31, 2021

santigimeno added the not-stale Issues that should never be marked stale label Jun 3, 2021

santigimeno approved these changes Jun 8, 2021

View reviewed changes

vtjnash merged commit 99eb736 into libuv:v1.x Jun 10, 2021

vtjnash deleted the jn/tcp-close-win-rst branch June 10, 2021 17:12

twose mentioned this pull request Apr 1, 2022

win: fix unexpected ECONNRESET error on TCP socket #3584

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

win,tcp: make uv_close work more like unix #3036

win,tcp: make uv_close work more like unix #3036

vtjnash commented Nov 4, 2020

vtjnash commented Nov 5, 2020

vtjnash commented Nov 10, 2020

mmomtchev commented Nov 12, 2020

vtjnash commented Nov 12, 2020

vtjnash commented Nov 12, 2020

vtjnash commented Nov 12, 2020

mmomtchev commented Nov 12, 2020

vtjnash commented Nov 13, 2020

stale bot commented Dec 4, 2020

vtjnash commented May 10, 2021

vtjnash commented May 20, 2021

vtjnash commented May 24, 2021

vtjnash commented May 27, 2021

vtjnash commented May 31, 2021

vtjnash commented Jun 1, 2021

vtjnash commented Jun 2, 2021 •

edited

vtjnash commented Jun 3, 2021

vtjnash commented Jun 5, 2021

vtjnash commented Jun 8, 2021

santigimeno left a comment

santigimeno Jun 8, 2021

vtjnash Jun 8, 2021

santigimeno Jun 8, 2021

vtjnash Jun 8, 2021

twose commented Mar 30, 2022

vtjnash commented Mar 31, 2022

vtjnash commented Mar 31, 2022

twose commented Mar 31, 2022

vtjnash commented Mar 31, 2022

win,tcp: make uv_close work more like unix #3036

win,tcp: make uv_close work more like unix #3036

Conversation

vtjnash commented Nov 4, 2020

vtjnash commented Nov 5, 2020

vtjnash commented Nov 10, 2020

mmomtchev commented Nov 12, 2020

vtjnash commented Nov 12, 2020

vtjnash commented Nov 12, 2020

vtjnash commented Nov 12, 2020

mmomtchev commented Nov 12, 2020

vtjnash commented Nov 13, 2020

stale bot commented Dec 4, 2020

vtjnash commented May 10, 2021

vtjnash commented May 20, 2021

vtjnash commented May 24, 2021

vtjnash commented May 27, 2021

vtjnash commented May 31, 2021

vtjnash commented Jun 1, 2021

vtjnash commented Jun 2, 2021 • edited

vtjnash commented Jun 3, 2021

vtjnash commented Jun 5, 2021

vtjnash commented Jun 8, 2021

santigimeno left a comment

Choose a reason for hiding this comment

santigimeno Jun 8, 2021

Choose a reason for hiding this comment

vtjnash Jun 8, 2021

Choose a reason for hiding this comment

santigimeno Jun 8, 2021

Choose a reason for hiding this comment

vtjnash Jun 8, 2021

Choose a reason for hiding this comment

twose commented Mar 30, 2022

vtjnash commented Mar 31, 2022

vtjnash commented Mar 31, 2022

twose commented Mar 31, 2022

vtjnash commented Mar 31, 2022

vtjnash commented Jun 2, 2021 •

edited