Repository navigation
tcp: free the TLS object when a client dies inside its loop - #62
Merged
Merged
Conversation
connect_wait() kept the connect callback's result in stack locals. When the TCP connect timed out, the function returned while the connect was still pending; ~uvcpp_http_client then pumped the loop to close the handle, the late callback fired, and it wrote into the dead stack frame (SIGSEGV in ~uvcpp_http_client / server_client::request). Keep that state on the heap so a callback that outlives connect_wait writes somewhere valid. Reproduced with molink-core pointed at an unreachable HTTPS directory: it segfaulted within seconds before, and now stays up across repeated retries.
write_stream_data() appended to an unbounded std::string. A peer that stops reading fills its QUIC window, nghttp3 stops consuming, and the relay can keep appending until the process is OOM-killed. Reject beyond 4 MiB so the caller can drop the message (MOLK repairs via tile clock). This is after the v1.8.7 tag; release it as a later version and then re-enable the server's HTTP/3 relay endpoint.
The running-loop branch of ~uvcpp_tcp_client() did `tls_ssl_ = nullptr;` and never deleted the uvcpp_ssl allocated by enable_tls(). On a server that accepts TLS connections this leaks one uvcpp_ssl plus its OpenSSL state per connection: valgrind reported 1,172,840 bytes in 20 blocks (20 TLS connections) definitely lost, allocated in enable_tls <- finish_accept, and a live 0.3.4 server grew 12-14 MB/min at idle while its clients opened short-lived HTTPS connections. Keep the pointer in a local and settle it by handle state: delete it immediately when the TCP handle is already detached (no TLS callback can be on the stack), hand it to the existing tcp_->close() completion when the handle is alive (that callback runs after the current frame unwinds), and only leave it behind in the rare "close slot already taken" branch that already logs a wrapper leak. The non-running branch and the no-handle case are safe to free on the spot. Verified with 20 TLS connections under valgrind (the 20 leaked blocks are gone) and with 500 fresh TLS connections against a server: RSS grew ~26 MB before and ~1.3 MB after.
`78eb5e0` 在 `uvcpp_http_client.cpp` 的 connect_wait 里净插 9 行、`b8b29b9` 在 `uvcpp_tcp_client.cpp` 的析构里净插 36 行,两条腿之后的引用整体后移。 4 条 `[歧]`(同一句 `last_error_code_ = UV_ENOTSUP;` / `HTTP_STATUS_NONE` 在文件里出现多次)按**统一平移量**定:471→480、753→762、1511→1520、 1614→1623;另外 `762→771` 是被重复内容掩盖、工具不会报的一条,按同一规则 一起改了(`web-http-guide.md:529` 现在引的是四个**不同**的 ENOTSUP 点)。 锁文件里只有一条键没动而哈希变了(`uvcpp_http_client.cpp:480`)——那正是 上面 471→480 的那一条,已人工核对。
- 版本头 PATCH 8;两版 README 的徽章与 Version 行同步(下载表等资产出来再换)。 - CHANGELOG:补上 **v1.8.7 那一节**(发布时只写了 RELEASE.md,逐个版本里缺账) 与 v1.8.8 一节;主题清单里记三条 —— 1.8.7 的 connect_wait 崩溃与 h3 发送 队列上限、1.8.8 的 TLS 对象泄漏;当前源码树/补丁版清单/tag 清单同步到 1.8.8。 - RELEASE.md:新增「v1.8.8 重点」。 本地:check_docs / check_doc_lines / check_doc_versions 全绿; release_notes.py --tag v1.8.8 能切出正文。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR contains
This branch carries the pending 1.8.7 transport line plus one new fix:
78eb5e0— Fix a crash in http_client when the directory is unreachable4000484— docs: bump README version to 1.8.7e76f693— h3: bound an extended-CONNECT stream's outbound queuee67f731— docs: add the v1.8.7 release notesb8b29b9— tcp: free the TLS object when a client dies inside its loop (new)The rest of this description covers commit 5.
Problem
~uvcpp_tcp_client()takes the "loop is running" branch for every clientdestroyed from a server close callback, and that branch did:
tls_ssl_ = nullptr;without deleting the
uvcpp_sslallocated byenable_tls(). Every acceptedTLS connection therefore leaked one
uvcpp_sslplus its OpenSSL state.Measured on a live moLink Server 0.3.4 (device + admin listeners TLS):
1,172,840 (1,280 direct, 1,171,560 indirect) bytes in 20 blocks definitely lost, allocated inuvcpp_tcp_client::enable_tls←uvcpp_tcp_server::finish_accept←uvcpp_stream::callback_connection.GET /healthz: RSS +26 MB, private +31 MB.The same 500 requests over one keep-alive TLS connection, or over plain
HTTP, are flat.
clients heartbeat over short-lived HTTPS connections.
Fix
Keep the pointer in a local and settle it by handle state:
can still be on the stack;
uv_closecan be scheduled: hand it to the existingtcp_->close(...)completion, which runs after the current frame unwinds;wrapper leak).
Verification
one-off 64 KiB read buffer remains at shutdown).
moLink server, where the memory curve is flat at ~17–19 MB under real
traffic.