unix: don't send handle twice on partial write

Guard against sending the handle over the UNIX domain socket twice
when the first sendmsg() didn't write all bytes.

The changes to src/win partially undo changes made earlier this year,
see the referenced pull request for details.

Libuv never made promises about the value of `req->send_handle` at
different points in time so this should be a safe, non-breaking change.

No tests because this particular condition is hard to hit reliably
across platforms. I spent a lot of time trying to write one but it
turned out hideously complex, and worse, flaky.

Fixes: https://github.com/libuv/libuv/issues/2086
PR-URL: https://github.com/libuv/libuv/pull/2097
Refs: https://github.com/libuv/libuv/pull/1843
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
This commit is contained in:
Ben Noordhuis 2018-12-30 19:35:10 +01:00
parent 639cc46f2c
commit c560cf931c
3 changed files with 7 additions and 9 deletions

View File

@ -882,6 +882,10 @@ start:
do
n = sendmsg(uv__stream_fd(stream), &msg, 0);
while (n == -1 && RETRY_ON_WRITE_ERROR(errno));
/* Ensure the handle isn't sent again in case this is a partial write. */
if (n >= 0)
req->send_handle = NULL;
} else {
do
n = uv__writev(uv__stream_fd(stream), iov, iovcnt);

View File

@ -1310,7 +1310,6 @@ static int uv__pipe_write_data(uv_loop_t* loop,
uv_pipe_t* handle,
const uv_buf_t bufs[],
size_t nbufs,
uv_stream_t* send_handle,
uv_write_cb cb,
int copy_always) {
int err;
@ -1321,7 +1320,7 @@ static int uv__pipe_write_data(uv_loop_t* loop,
UV_REQ_INIT(req, UV_WRITE);
req->handle = (uv_stream_t*) handle;
req->send_handle = send_handle;
req->send_handle = NULL;
req->cb = cb;
/* Private fields. */
req->coalesced = 0;
@ -1558,8 +1557,7 @@ int uv__pipe_write_ipc(uv_loop_t* loop,
/* Write buffers. We set the `always_copy` flag, so it is not a problem that
* some of the written data lives on the stack. */
err = uv__pipe_write_data(
loop, req, handle, bufs, buf_count, send_handle, cb, 1);
err = uv__pipe_write_data(loop, req, handle, bufs, buf_count, cb, 1);
/* If we had to heap-allocate the bufs array, free it now. */
if (bufs != stack_bufs) {
@ -1583,8 +1581,7 @@ int uv__pipe_write(uv_loop_t* loop,
} else {
/* Non-IPC pipe write: put data on the wire directly. */
assert(send_handle == NULL);
return uv__pipe_write_data(
loop, req, handle, bufs, nbufs, NULL, cb, 0);
return uv__pipe_write_data(loop, req, handle, bufs, nbufs, cb, 0);
}
}

View File

@ -149,7 +149,6 @@ static void connect_cb(uv_connect_t* req, int status) {
&ctx.send.stream,
NULL);
ASSERT(r == 0);
ASSERT(ctx.write_req.send_handle == &ctx.send.stream);
/* Perform two writes to the same pipe to make sure that on Windows we are
* not running into issue 505:
@ -161,7 +160,6 @@ static void connect_cb(uv_connect_t* req, int status) {
&ctx.send2.stream,
NULL);
ASSERT(r == 0);
ASSERT(ctx.write_req2.send_handle == &ctx.send2.stream);
r = uv_read_start((uv_stream_t*)&ctx.channel, alloc_cb, recv_cb);
ASSERT(r == 0);
@ -346,7 +344,6 @@ static void read_cb(uv_stream_t* handle,
&recv->stream,
write2_cb);
ASSERT(r == 0);
ASSERT(write_req->send_handle == &recv->stream);
} while (uv_pipe_pending_count(pipe) > 0);
}