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 417c6cb ("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 tarantool#9346
Fixes tarantool#9453

NO_DOC=bugfix

changelogs/unreleased/gh-9346-crash-on-GC-collection-of-http-objects.md

@@ -0,0 +1,3 @@
+## bugfix/http
+
+* Fixed a crash due to a race in GC finalizers (gh-9346).

changelogs/unreleased/gh-9453-http-client-is-gc-collected-when-chunked-request-is-alive.md

@@ -0,0 +1,4 @@
+## bugfix/http
+
+* Fixed behaviour of GC-finalizer of HTTP client Lua object when chunked HTTP
+  request is alive. (gh-9453).

src/curl.c

@@ -259,7 +259,7 @@ curl_env_create(struct curl_env *env, long max_conns, long max_total_conns)
 }
 
 void
-curl_env_destroy(struct curl_env *env)
+curl_env_finish(struct curl_env *env)
 {
 	assert(env);
 	if (env->multi != NULL) {
@@ -276,6 +276,15 @@ curl_env_destroy(struct curl_env *env)
 		}
 		curl_multi_cleanup(env->multi);
 	}
+}
+
+void
+curl_env_destroy(struct curl_env *env)
+{
+	assert(env);
+
+	assert(mempool_count(&env->sock_pool) == 0);
+	assert(mempool_used(&env->sock_pool) == 0);
 
 	mempool_destroy(&env->sock_pool);
 }

src/curl.h

@@ -111,6 +111,13 @@ curl_env_create(struct curl_env *env, long max_conns, long max_total_conns);
 void
 curl_env_destroy(struct curl_env *env);
 
+/**
+ * Finish HTTP client environment
+ * @param env pointer to an environment to finish
+ */
+void
+curl_env_finish(struct curl_env *env);
+
 /**
  * Initialize a new CURL request
  * @param curl_request request

src/httpc.c

@@ -140,6 +140,15 @@ httpc_env_create(struct httpc_env *env, int max_conns, int max_total_conns)
 	return curl_env_create(&env->curl_env, max_conns, max_total_conns);
 }
 
+void
+httpc_env_finish(struct httpc_env *ctx)
+{
+	assert(ctx);
+
+	curl_env_finish(&ctx->curl_env);
+	ctx->cleanup = 1;
+}
+
 void
 httpc_env_destroy(struct httpc_env *ctx)
 {
@@ -208,12 +217,18 @@ httpc_request_new(struct httpc_env *env, const char *method,
 
 	ibuf_create(&req->send, &cord()->slabc, 1);
 
+	++env->req_count;
+
 	return req;
 }
 
 void
 httpc_request_delete(struct httpc_request *req)
 {
+	struct httpc_env *env = req->env;
+	--env->req_count;
+	assert(env->req_count >= 0);
+
 	if (req->headers != NULL)
 		curl_slist_free_all(req->headers);
 
@@ -230,6 +245,11 @@ httpc_request_delete(struct httpc_request *req)
 	}
 
 	mempool_free(&req->env->req_pool, req);
+
+	if (env->req_count == 0 &&
+	    env->cleanup == 1) {
+		httpc_env_destroy(env);
+	}
 }
 
 int

src/httpc.h

@@ -60,6 +60,10 @@ struct httpc_stat {
  * HTTP Client Environment
  */
 struct httpc_env {
+	/** Number of a current active http requests. */
+	int req_count;
+	/** Flag that triggers cleanup. */
+	int cleanup;
 	/** Curl environment. */
 	struct curl_env curl_env;
 	/** Memory pool for requests */
@@ -78,6 +82,13 @@ struct httpc_env {
 int
 httpc_env_create(struct httpc_env *ctx, int max_conns, int max_total_conns);
 
+/**
+ * Finish HTTP client environment
+ * @param env pointer to a structure to finish
+ */
+void
+httpc_env_finish(struct httpc_env *env);
+
 /**
  * Destroy HTTP client environment
  * @param env pointer to a structure to destroy

src/lua/httpc.c

@@ -504,7 +504,11 @@ luaT_httpc_new(lua_State *L)
 static int
 luaT_httpc_cleanup(lua_State *L)
 {
-	httpc_env_destroy(luaT_httpc_checkenv(L));
+	struct httpc_env *env = luaT_httpc_checkenv(L);
+	httpc_env_finish(env);
+	if (env->req_count == 0) {
+		httpc_env_destroy(env);
+	}
 
 	/** remove all methods operating on ctx */
 	lua_newtable(L);
@@ -607,7 +611,8 @@ luaT_httpc_io_finish(lua_State *L)
 static int
 luaT_httpc_io_cleanup(lua_State *L)
 {
-	httpc_io_destroy(luaT_httpc_checkio(L));
+	struct httpc_io *io = luaT_httpc_checkio(L);
+	httpc_io_destroy(io);
 
 	/** remove all methods operating on io */
 	lua_newtable(L);

src/lua/httpc.lua

@@ -614,6 +614,9 @@ curl_mt = {
                         write = io_write,
                         finish = io_finish,
                     },
+                    _internal = {
+                        httpc = self,
+                    },
                 })
             end
         end,
@@ -751,5 +754,6 @@ for _, name in ipairs({ 'get', 'delete', 'trace', 'options', 'head',
                      'connect', 'post', 'put', 'patch', 'request'}) do
     this_module[name] = http_default_wrap(name)
 end
+this_module.curl = http_default.curl
 
 return this_module

test/app-luatest/http_client_test.lua

@@ -1375,3 +1375,49 @@ g.test_http_client_io_post_parts = function(cg)
     t.assert_equals(io.status, 200, 'io')
     t.assert_equals(io.reason, 'Ok', '200 - Ok')
 end
+
+-- GH-9453: GC had collected HTTP client object
+-- when HTTP IO object is alive.
+-- Symptom: A message "IllegalParams: io: request must be io"
+-- is happened on calling HTTP IO methods.
+g.test_http_client_gc_finalizer_gh_9453 = function(cg)
+    local url, opts = cg.url, table.deepcopy(cg.opts)
+    local client = cg.client.new()
+    opts.chunked = true
+
+    local http_io = client:get(url, opts)
+
+    -- GC collect HTTP client object.
+    client = nil -- luacheck: no unused
+    collectgarbage()
+    collectgarbage()
+
+    -- HTTP IO object is alive.
+    local ok, res = pcall(http_io.read, http_io, 4)
+    t.assert_equals(ok, true)
+    t.assert_equals(res, 'hell')
+
+    -- Teardown.
+    http_io = nil -- luacheck: no unused
+    collectgarbage()
+    collectgarbage()
+end
+
+g.test_gh_9346_httpc_io_cleanup = function(cg)
+    local url, opts = cg.url, table.deepcopy(cg.opts)
+    local client = cg.client.new()
+    opts.chunked = true
+
+    local http_io = client:get(url, opts) -- luacheck: no unused
+
+    collectgarbage()
+    collectgarbage()
+
+    -- Teardown.
+    client = nil -- luacheck: no unused
+    collectgarbage()
+    collectgarbage()
+    http_io = nil -- luacheck: no unused
+    collectgarbage()
+    collectgarbage()
+end