mirror of
https://github.com/neovim/neovim.git
synced 2025-11-16 07:11:20 +00:00
fix(socket): avoid stack-use-after-return after timeout (#36405)
Problem: In socket_connect(), if connecting to the given TCP address times out, libuv is still trying to connect to the address, and connect_cb may be called when running the libuv event loop after the `status` variable referenced by `req.data` goes out of scope. Solution: Close the uv_tcp_t handle and wait for connect_cb to be called before retrying or failing in socket_connect(). This also avoid leaking libuv handles. The tests added here only check that the non-timeout case still works, as checking the timeout case is very hard without modifications to the code. Removing the first LOOP_PROCESS_EVENT_UNTIL() in socket_connect() (the one with the timeout) is a way to check that manually. Also add a comment about the cause of the ASAN error in #34586.
This commit is contained in:
@@ -190,8 +190,9 @@ static void connect_cb(uv_connect_t *req, int status)
|
|||||||
{
|
{
|
||||||
int *ret_status = req->data;
|
int *ret_status = req->data;
|
||||||
*ret_status = status;
|
*ret_status = status;
|
||||||
if (status != 0) {
|
uv_handle_t *handle = (uv_handle_t *)req->handle;
|
||||||
uv_close((uv_handle_t *)req->handle, NULL);
|
if (status != 0 && !uv_is_closing(handle)) {
|
||||||
|
uv_close(handle, NULL);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -245,12 +246,24 @@ tcp_retry:
|
|||||||
if (status == 0) {
|
if (status == 0) {
|
||||||
stream_init(NULL, &stream->s, -1, uv_stream);
|
stream_init(NULL, &stream->s, -1, uv_stream);
|
||||||
success = true;
|
success = true;
|
||||||
} else if (is_tcp && addrinfo->ai_next) {
|
} else {
|
||||||
|
if (!uv_is_closing((uv_handle_t *)uv_stream)) {
|
||||||
|
uv_close((uv_handle_t *)uv_stream, NULL);
|
||||||
|
if (status == 1) {
|
||||||
|
// The uv_close() above will make libuv call connect_cb() with UV_ECANCELED.
|
||||||
|
// Make sure connect_cb() has been called here, as if it's called after this
|
||||||
|
// function ends it will cause a stack-use-after-scope.
|
||||||
|
LOOP_PROCESS_EVENTS_UNTIL(&main_loop, NULL, -1, status != 1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (is_tcp && addrinfo->ai_next) {
|
||||||
addrinfo = addrinfo->ai_next;
|
addrinfo = addrinfo->ai_next;
|
||||||
goto tcp_retry;
|
goto tcp_retry;
|
||||||
} else {
|
} else {
|
||||||
*error = _("connection refused");
|
*error = _("connection refused");
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
cleanup:
|
cleanup:
|
||||||
xfree(addr);
|
xfree(addr);
|
||||||
|
|||||||
@@ -142,6 +142,8 @@ void stream_close_handle(Stream *stream)
|
|||||||
static void close_cb(uv_handle_t *handle)
|
static void close_cb(uv_handle_t *handle)
|
||||||
{
|
{
|
||||||
Stream *stream = handle->data;
|
Stream *stream = handle->data;
|
||||||
|
// Need to check if handle->data is NULL here as this callback may be called between
|
||||||
|
// the handle's initialization and stream_init() (e.g. in socket_connect()).
|
||||||
if (stream && stream->close_cb) {
|
if (stream && stream->close_cb) {
|
||||||
stream->close_cb(stream, stream->close_cb_data);
|
stream->close_cb(stream, stream->close_cb_data);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -465,6 +465,22 @@ describe('channels', function()
|
|||||||
eq(true, exec_lua('return _G.result'))
|
eq(true, exec_lua('return _G.result'))
|
||||||
end)
|
end)
|
||||||
end)
|
end)
|
||||||
|
|
||||||
|
describe('sockconnect() reports error when connection fails', function()
|
||||||
|
it('in "pipe" mode', function()
|
||||||
|
eq(
|
||||||
|
'Vim:connection failed: connection refused',
|
||||||
|
pcall_err(fn.sockconnect, 'pipe', n.new_pipename())
|
||||||
|
)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('in "tcp" mode', function()
|
||||||
|
eq(
|
||||||
|
'Vim:connection failed: connection refused',
|
||||||
|
pcall_err(fn.sockconnect, 'pipe', '127.0.0.1:0')
|
||||||
|
)
|
||||||
|
end)
|
||||||
|
end)
|
||||||
end)
|
end)
|
||||||
|
|
||||||
describe('loopback', function()
|
describe('loopback', function()
|
||||||
|
|||||||
Reference in New Issue
Block a user