Commits


httpc: fix a race in GC finalizers `httpc` module has two GC-finalizers: the first one for a Lua http client (C function `luaT_httpc_cleanup`) and the second one for a Lua http chunked requests (C function `luaT_httpc_io_cleanup`) introduced in commit 417c6cb7b0c6 ("httpc: introduce stream input/output interface"). In a C implementation HTTP requests depends on structures of HTTP client and there is a problem with destroying Lua objects in `httpc` module - these GC-finalizers are not synchronized. This could lead to at least two problems: There is a race with GC-finalization that leads to use-after-free errors when HTTP client is collected before collecting HTTP request. In a stacktrace the problem looks as below: ``` 0x55ca7d47652e in crash_collect+256 0x55ca7d476f6a in crash_signal_cb+100 0x7fb876c42520 in __sigaction+80 0x55ca7d641e51 in curl_slist_free_all+35 0x55ca7d441498 in httpc_request_delete+45 0x55ca7d4653f1 in httpc_io_destroy+27 0x55ca7d4674bc in luaT_httpc_io_cleanup+36 0x55ca7d4e00c7 in lj_BC_FUNCC+70 0x55ca7d4f8364 in gc_call_finalizer+668 0x55ca7d4f8946 in gc_finalize+1387 0x55ca7d4f91e2 in gc_onestep+864 0x55ca7d4f9716 in lj_gc_fullgc+276 ... ``` Lua object `http.client` could be GC-collected when chunked HTTP request is alive. This will lead to an error "IllegalParams: io: request must be io" because we call a method when Lua object is already a `nil`. ```lua local url = 'https://bronevichok.ru/' local c = require('http.client').new() local r = c:get(url, {chunked = true}) c = nil collectgarbage() collectgarbage() r:read(1) -- IllegalParams: io: request must be io ``` The patch introduces two functions: `httpc_env_finish` and `curl_env_finish`, that prepares curl and httpc environments for destruction. HTTP client's GC finalizer now calls `httpc_env_finish` instead of `httpc_env_destroy`, this prevents from destroying memory that could be in use by HTTP requests. Additionally `httpc_env_finish` sets a flag `cleanup`. HTTP environment destroying is called when flag `cleanup` is set and a there are no active HTTP requests. The main idea of the patch is a synchronization of destructors for HTTP client and HTTP chunked requests. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a Curl's userdata in Lua objects with HTTP chunked requests and HTTP default client. Fixes #9346 Fixes #9453 NO_DOC=bugfix


httpc: fix a crash triggered by gc Bump curl version to 8.4.0 triggers a crash in Tarantool due to commit "h2: testcase and fix for pausing h2 streams" [1]. The original reproducer involves etcd and an etcd-client Lua module, running etcd-client tests as a part of Tarantool integration testing is planned to do in scope of [1]. However, the problem could be reproduced with a Lua code below: ``` local url = 'https://google.com/' local c = require('http.client').new() r1 = c:get(url, {chunked = true}) r1:read(1) r2 = c:get(url, {chunked = true}) r2:read(1) r3 = c:get(url, {chunked = true}) r3:read(1) r4 = c:get(url, {chunked = true}) r4:read(1) c = nil collectgarbage() collectgarbage() r1:read(1) r2:read(1) r3:read(1) r4:read(1) collectgarbage() collectgarbage() ``` According to Curl documentation, `curl_multi_cleanup` [1] must be called before any easy handles are cleaned up. The patch adds a cleanup of easy handles on running `curl_env_destroy`, right before calling `curl_multi_cleanup`. The patch uses a function 'curl_multi_get_handles' that returns all added easy handles introduced in Curl 8.4.0. Therefore bump to 8.4.0 is required. 1. https://github.com/curl/curl/commit/6b9a591bf7d82031f463373706d7de1cba0adee6 2. https://curl.se/libcurl/c/curl_multi_cleanup.html Fixes #9283 1. https://github.com/tarantool/tarantool/issues/9093 NO_DOC=bugfix NO_TEST=no simple reproducer, covered by tests in etcd-client


httpc: prefer curl headers in submodule by default FreeBSD instances in Tarantool CI have installed libcurl package (as a dependency of Zabbix monitoring agent). Curl 8.4.0 introduces a new function `curl_multi_get_handles` that is used in the following commit in `src/curl.c`, but libcurl system package has no such symbol in headers. On building on FreeBSD in Tarantool CI C compiler produces a warning about implicit declaration of function, because it looks at system headers by default and due to enabled CMake option `-DENABLE_WERROR=ON` building has failed: ``` [ 63%] Building C object src/CMakeFiles/server.dir/title.c.o /.cache/act/55d136250dd94303/hostexecutor/src/curl.c:266:17: error: implicit declaration of function 'curl_multi_get_handles' is invalid in C99 [-Werror,-Wimplicit-function-declaration] CURL **list = curl_multi_get_handles(env->multi); ^ /.cache/act/55d136250dd94303/hostexecutor/src/curl.c:266:17: note: did you mean 'curl_multi_add_handle'? /usr/local/include/curl/multi.h:140:23: note: 'curl_multi_add_handle' declared here CURL_EXTERN CURLMcode curl_multi_add_handle(CURLM *multi_handle, ^ /.cache/act/55d136250dd94303/hostexecutor/src/curl.c:266:10: error: incompatible integer to pointer conversion initializing 'CURL **' (aka 'void **') with an expression of type 'int' [-Werror,-Wint-conversion] CURL **list = curl_multi_get_handles(env->multi); ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2 errors generated. ``` The patch fixes that by reordering headers passed to compiler, see [1]. 1. https://cmake.org/cmake/help/latest/command/include_directories.html Needed for #9283 NO_CHANGELOG=build NO_DOC=build NO_TEST=build


third_party: update libcurl from 8.3.0 to 8.4.0 The patch updates curl module to the version 8.4.0 [1] that brings a number of functional fixes and security fix of SOCKS5 heap buffer overflow (CVE-2023-38545), see description in [2] and commit fb4415d8aee6 ("socks: return error if hostname too long for remote resolve") in [3]. 1. https://curl.se/changes.html#8_4_0 2. https://curl.se/docs/CVE-2023-38545.html 3. https://github.com/curl/curl/commit/fb4415d8aee6c1045be932a34fe6107c2f5ed147 NO_DOC=libcurl submodule bump NO_TEST=libcurl submodule bump


tests: suppress message 'Broken pipe exception handling' Message below is printed every time on shutdown `httpd.py` when `test/app-luatest/http_client_test.lua` is running by luatest without capturing stdout: ``` BrokenPipeError: [Errno 32] Broken pipe exception handling ``` The patch suppress this exception by adding a handler for a signal `SIGPIPE`. NO_CHANGELOG=testing NO_DOC=testing NO_TEST=testing