New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
http client object is GC-collected when http request object is alive #9453
Labels
bug
Something isn't working
Comments
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 7, 2023
`http.client` has an incorrect behaviour: HTTP-client object could be GC-collected before GC-collecting an active chunked HTTP request. The patch adds a test with an incorrect behaviour. Following commits will fix this behaviour. Needed for tarantool#9453 NO_CHANGELOG=testing NO_DOC=testing
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 7, 2023
There is a problem with destroying http objects in `httpc` module triggered by GC. `httpc` module has two GC-finalizators: first one for a Lua http client (`luaT_httpc_cleanup`) and second one for a Lua http io (`luaT_httpc_io_cleanup`). In some circumstances GC-finalizators have a race that leads to use-after-free errors. 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 ... ``` The patch introduces two functions: `httpc_env_finish` and `curl_env_finish`, that prepares curl and httpc environments for destruction. HTTP client's GC finilizer 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 all HTTP requests were deleted. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 7, 2023
`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`). 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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 7, 2023
`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`). 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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 7, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 7, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 7, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 7, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 8, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 11, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 11, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 12, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 12, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 12, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 14, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 14, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 15, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 15, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 18, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 18, 2023
`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 descructors for HTTP client and HTTP chunked request. Unfortunately, GC will eventually collect HTTP client object after calling its `__gc`. To prevent this we put a reference to a userdata with HTTP client object to a `LUA_REGISTRYINDEX` and remove this reference only when GC-finalizer for HTTP client is called and there are no active HTTP chunked requests. Fixes tarantool#9346 Fixes tarantool#9453 NO_DOC=bugfix
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 18, 2023
`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
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 18, 2023
`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
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 19, 2023
`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
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Dec 19, 2023
`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
ligurio
added a commit
to ligurio/tarantool
that referenced
this issue
Jan 9, 2024
`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 (cherry picked from commit 17e9c6f)
igormunkin
pushed a commit
that referenced
this issue
Jan 10, 2024
`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 #9346 Fixes #9453 NO_DOC=bugfix (cherry picked from commit 17e9c6f)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
(initially reported by Oleg Babin in #9341 (comment))
Bug description
http client object is GC-collected when http request object is alive
Place
tarantool --version
output here.Steps to reproduce
reproducer script:
How to run:
Just type
tarantool test.lua
.Actual behavior
Expected behavior
Successful read.
The text was updated successfully, but these errors were encountered: