Possible `Multi` wrapper retention after `close()`
- Dominant language
- TypeScript
- Stars
- 718
- Forks
- 135
- PR merge metrics
- No merged PRs in 30d
Description
# Possible `Multi` wrapper retention after `close()`
I found a possible lifecycle leak where `Multi::close()` releases the curl multi handle but leaves the wrapper and timer alive until environment shutdown.
File: `src/Multi.cc`
Functions: `Multi::Multi`, `Multi::Close`, `Multi::Dispose`, `Multi::CloseTimerAsync`
Relevant code:
```cpp
uv_timer_init(loop, &this->timeout);
this->timeout.data = this;
// We need to keep the reference alive for the duration of the timer.
this->Ref();
napi_add_async_cleanup_hook(env, Multi::CleanupHookAsync, this, &removeHandle);
```
The reference is released only from the `uv_close` callback:
```cpp
uv_close(timeoutHandle, [](uv_handle_t* handle) {
uv_timer_t* timer = reinterpret_cast(handle);
Multi* multi = static_cast(timer->data);
napi_remove_async_cleanup_hook(multi->removeHandle);
multi->Unref();
});
```
But public `close()` only calls `Dispose()`:
```cpp
Napi::Value Multi::Close(const Napi::CallbackInfo& info) {
// ...
this->Dispose();
return env.Undefined();
}
```
`Dispose()` stops the timer and cleans up the curl multi handle, but does not
close the uv handle or release the strong wrapper reference:
```cpp
this->isOpen = false;
uv_timer_stop(&this->timeout);
if (this->mh) {
CURLMcode code = curl_multi_cleanup(this->mh);
assert(code == CURLM_OK);
this->mh = nullptr;
}
```
`CloseTimerAsync()` is called by the environment cleanup hook, so an explicitly
closed `Multi` can remain reachable through its own `Ref()` until environment
teardown.
Suggested fix: make `Multi::Close()` also initiate the timer close path, for
example by calling `CloseTimerAsync()` after `Dispose()`, while preserving the
existing duplicate-close guards.
Contributor guide
Research direction
Start in src/Multi.cc by tracing Multi::Close, Multi::Dispose, Multi::CloseTimerAsync, and the constructor's timer setup. Confirm how explicit close interacts with the existing cleanup hook and duplicate-close guards; done means the wrapper is no longer retained after close while environment cleanup remains safe.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, node.js
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100