tarantool / tarantool / 7300160427

88%

Build Type

push

github

Committed by igormunkin

Commit Message

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 envi... (continued)

Run Details

66190 of 118469 branches covered (0.0%)

23 of 24 new or added lines in 3 files covered. (95.83%)

57 existing lines in 15 files now uncovered.

98389 of 113324 relevant lines covered (86.82%)

2673871.58 hits per line