Skip to content

Commit b3a0736

Browse files
committed
fix(sdk/cpp)+test: TCPTransport worker 池生命周期修复——Close 挂起与 move 后 terminate
测试暴露的两个产品 bug(test_provider_inbound 用例曾 100% 挂死): 1. Close() 挂起 + use-after-free:inbound worker for(;;) 永不退出, ~TCPTransport 销毁被等待中的 mutex/cv(UB 挂起);析构还会二次 Close(显式 Close 后 transport_.reset())重复操作 socket/线程。 修复:池共享状态移入 shared_ptr<InboundPool>(worker 持 pool 引用 而非 this,socket fd 值拷贝、关闭后 send 仅 EBADF);Close 置 stopping + notify_all + detach(Go MuxConn 同款不等待语义); close_called_ 幂等守卫防双跑。 2. move 后 terminate:守卫使接管对象的析构跳过 Close,被 move 进来 的 read_thread_ 永不 join → std::terminate。move ctor/assignment 正确转移 close_called_/inbound_pool_ 并将源置已关闭。 补测 test_provider_inbound(6+1 用例,原生 fake agent 主动推请求—— TCPServer 只能应答无法触发入站路径): - agent invoke 到达 handler 并回带 payload 响应 - 未注册函数回空体;垃圾 protobuf 被捕获且连接仍可用 - ProviderHeartbeat pong - 16 并发慢 handler 全应答(worker 池 + 队列语义) - 非法地址(缺端口/IPv6 畸形/端口越界/http scheme)Connect 拒绝 覆盖:croupier_client.cpp 79%→84%、tcp_transport.cpp 77%→86%。 ctest 全绿。
1 parent 7d5c1db commit b3a0736

5 files changed

Lines changed: 451 additions & 46 deletions

File tree

sdks/cpp/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,6 +696,7 @@ if(BUILD_TESTS)
696696
tests/test_openapi_importer.cpp
697697
tests/test_coverage_boost2.cpp
698698
tests/test_coverage_boost3.cpp
699+
tests/test_provider_inbound.cpp
699700
)
700701

701702
# Real shared-object plugins used by the dynamic loader tests.

sdks/cpp/include/croupier/sdk/tcp_transport.h

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ class TCPTransport {
116116

117117
void SetInboundHandler(InboundHandler handler);
118118
void DispatchInbound(uint32_t msg_id, uint32_t req_id, std::vector<uint8_t> body);
119-
void WriteResponseSilently(uint32_t resp_msg_id, uint32_t req_id, const std::vector<uint8_t>& body);
119+
static void WriteResponseOnSocket(socket_t sock, uint32_t resp_msg_id, uint32_t req_id,
120+
const std::vector<uint8_t>& body);
120121
static int InboundWorkerCount();
121122

122123
/**
@@ -160,24 +161,36 @@ class TCPTransport {
160161
bool ConnectWithTimeout(int timeout_ms);
161162
void SetSocketNonBlocking(bool non_blocking);
162163

164+
// 入站 worker 池共享状态:worker 持 shared_ptr 而非 this——
165+
// Close/析构后 worker 仍可安全退出(socket fd 为值拷贝,关闭后
166+
// send 仅返回 EBADF),修复「worker for(;;) 永不退出 + 析构成员
167+
// 被等待中的线程使用」导致的挂起/use-after-free。
168+
struct InboundPool {
169+
std::mutex mu;
170+
std::condition_variable cv;
171+
std::queue<std::tuple<uint32_t, uint32_t, std::vector<uint8_t>>> queue;
172+
std::atomic<int> queued{0};
173+
bool started = false;
174+
bool stopping = false;
175+
std::vector<std::thread> threads;
176+
InboundHandler handler;
177+
socket_t sock = INVALID_SOCKET_VALUE;
178+
};
179+
163180
std::string host_;
164181
int port_;
165182
int timeout_ms_;
166183
int connect_timeout_ms_; // Separate timeout for connection attempts
167184
socket_t socket_;
168185
std::atomic<bool> connected_;
169186
std::atomic<bool> closing_;
187+
bool close_called_ = false; // Close() 幂等:显式 Close + 析构 Close 双跑防护
170188
std::atomic<uint32_t> next_req_id_;
171189
std::unordered_map<uint32_t, std::unique_ptr<ResponseLatch>> pending_responses_;
172190
std::mutex pending_mutex_;
173191
std::thread read_thread_;
174192
InboundHandler inbound_handler_;
175-
std::mutex inbound_pool_mutex_;
176-
std::vector<std::thread> inbound_workers_;
177-
std::queue<std::tuple<uint32_t, uint32_t, std::vector<uint8_t>>> inbound_queue_;
178-
std::condition_variable inbound_cv_;
179-
std::atomic<int> inbound_queued_{0};
180-
bool inbound_pool_started_ = false;
193+
std::shared_ptr<InboundPool> inbound_pool_;
181194

182195
static constexpr size_t FRAME_HEADER_BYTES = 4;
183196
static constexpr size_t PROTOCOL_HEADER_SIZE = 8;

sdks/cpp/src/croupier_client.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,6 @@ class CroupierClient::Impl {
531531
if (reconnect_thread_.joinable()) {
532532
reconnect_thread_.join();
533533
}
534-
535534
closeTransport();
536535
session_id_.clear();
537536

sdks/cpp/src/tcp_transport.cpp

Lines changed: 96 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,14 @@ TCPTransport::TCPTransport(TCPTransport&& other) noexcept
7171
closing_(other.closing_.load()),
7272
next_req_id_(other.next_req_id_.load()),
7373
pending_responses_(std::move(other.pending_responses_)),
74-
read_thread_(std::move(other.read_thread_)) {
74+
read_thread_(std::move(other.read_thread_)),
75+
inbound_pool_(std::move(other.inbound_pool_)) {
76+
// 接管方的 close_called_ 必须复位:否则其析构被幂等守卫跳过,
77+
// 被 move 进来的 read_thread_/pool 永不回收 → terminate。
78+
close_called_ = other.close_called_;
7579
other.socket_ = INVALID_SOCKET_VALUE;
7680
other.connected_ = false;
81+
other.close_called_ = true; // 源对象已掏空,析构 Close 直通 no-op
7782
}
7883

7984
TCPTransport& TCPTransport::operator=(TCPTransport&& other) noexcept {
@@ -90,9 +95,14 @@ TCPTransport& TCPTransport::operator=(TCPTransport&& other) noexcept {
9095
next_req_id_ = other.next_req_id_.load();
9196
pending_responses_ = std::move(other.pending_responses_);
9297
read_thread_ = std::move(other.read_thread_);
98+
inbound_pool_ = std::move(other.inbound_pool_);
9399

100+
// 目标接管存活连接:close_called_ 复位,析构可正常回收线程;
101+
// 源已掏空,置 true 让其析构 Close 直通 no-op。
102+
close_called_ = other.close_called_;
94103
other.socket_ = INVALID_SOCKET_VALUE;
95104
other.connected_ = false;
105+
other.close_called_ = true;
96106
}
97107
return *this;
98108
}
@@ -279,6 +289,12 @@ void TCPTransport::Connect() {
279289
}
280290

281291
void TCPTransport::Close() {
292+
// 幂等:显式 Close 与析构 Close 双跑防护(曾出现 socket/线程被
293+
// 二次关闭与 join 的未定义行为)。
294+
if (close_called_) {
295+
return;
296+
}
297+
close_called_ = true;
282298
closing_ = true;
283299
connected_ = false;
284300

@@ -314,6 +330,28 @@ void TCPTransport::Close() {
314330
read_thread_.join();
315331
}
316332

333+
// Inbound worker 池优雅停机:置 stopping + 唤醒全部 worker;worker
334+
// 引用 shared_ptr<InboundPool>,detach 后即使仍有 handler 在途,
335+
// pool 状态与 socket fd(值拷贝,send 仅得 EBADF)都保持有效——
336+
// 不会因 ~TCPTransport 而悬空。join 会阻塞在用户 handler 上,
337+
// 这里选择 detach(Go MuxConn 同款语义:不等待业务排空)。
338+
if (inbound_pool_) {
339+
{
340+
std::lock_guard<std::mutex> lock(inbound_pool_->mu);
341+
inbound_pool_->stopping = true;
342+
inbound_pool_->sock = INVALID_SOCKET_VALUE;
343+
inbound_pool_->handler = nullptr;
344+
}
345+
inbound_pool_->cv.notify_all();
346+
for (auto& t : inbound_pool_->threads) {
347+
if (t.joinable()) {
348+
t.detach();
349+
}
350+
}
351+
inbound_pool_->threads.clear();
352+
inbound_pool_.reset();
353+
}
354+
317355
// Clear any remaining responses (should be none after signaling)
318356
std::lock_guard<std::mutex> lock(pending_mutex_);
319357
pending_responses_.clear();
@@ -923,59 +961,78 @@ int TCPTransport::InboundWorkerCount() {
923961

924962
void TCPTransport::SetInboundHandler(InboundHandler handler) {
925963
inbound_handler_ = std::move(handler);
964+
// 已启动的池同步更新 handler(重连复用同一 transport 的场景)。
965+
if (inbound_pool_) {
966+
std::lock_guard<std::mutex> lock(inbound_pool_->mu);
967+
inbound_pool_->handler = inbound_handler_;
968+
}
926969
}
927970

928971
void TCPTransport::DispatchInbound(uint32_t msg_id, uint32_t req_id, std::vector<uint8_t> body) {
929972
if (!inbound_handler_) {
930973
return;
931974
}
932-
// 惰性启动固定 worker 池(默认 = 硬件并发数)
933-
{
934-
std::lock_guard<std::mutex> lock(inbound_pool_mutex_);
935-
if (!inbound_pool_started_) {
936-
int workers = InboundWorkerCount();
937-
for (int i = 0; i < workers; ++i) {
938-
inbound_workers_.emplace_back([this] {
939-
for (;;) {
940-
std::tuple<uint32_t, uint32_t, std::vector<uint8_t>> task;
941-
{
942-
std::unique_lock<std::mutex> lock(inbound_pool_mutex_);
943-
inbound_cv_.wait(lock, [this] { return !inbound_queue_.empty(); });
944-
task = std::move(inbound_queue_.front());
945-
inbound_queue_.pop();
975+
// 惰性启动固定 worker 池(默认 = 硬件并发数)。
976+
// worker 捕获 pool 的 shared_ptr(不捕获 this):Close/析构置 stopping
977+
// 后 worker 自行退出,pool 状态由最后一个引用负责释放——修复
978+
// 「worker for(;;) 永不退出、析构销毁被等待中的 mutex/cv」挂起。
979+
if (!inbound_pool_) {
980+
inbound_pool_ = std::make_shared<InboundPool>();
981+
inbound_pool_->handler = inbound_handler_;
982+
inbound_pool_->sock = socket_;
983+
const int workers = InboundWorkerCount();
984+
for (int i = 0; i < workers; ++i) {
985+
auto pool = inbound_pool_;
986+
inbound_pool_->threads.emplace_back([pool] {
987+
for (;;) {
988+
std::tuple<uint32_t, uint32_t, std::vector<uint8_t>> task;
989+
{
990+
std::unique_lock<std::mutex> lock(pool->mu);
991+
pool->cv.wait(lock, [&pool] { return !pool->queue.empty() || pool->stopping; });
992+
if (pool->queue.empty()) {
993+
return; // stopping 且无积压:退出
946994
}
947-
auto [mid, rid, tbody] = std::move(task);
948-
std::vector<uint8_t> resp;
949-
try {
950-
resp = inbound_handler_(mid, rid, tbody);
951-
} catch (const std::exception& e) {
952-
std::cerr << "[ERROR] inbound handler: " << e.what() << '\n';
953-
resp.clear();
995+
task = std::move(pool->queue.front());
996+
pool->queue.pop();
997+
}
998+
auto [mid, rid, tbody] = std::move(task);
999+
std::vector<uint8_t> resp;
1000+
try {
1001+
if (pool->handler) {
1002+
resp = pool->handler(mid, rid, tbody);
9541003
}
955-
WriteResponseSilently(protocol::GetResponseMsgID(mid), rid, resp);
956-
inbound_queued_.fetch_sub(1);
1004+
} catch (const std::exception& e) {
1005+
std::cerr << "[ERROR] inbound handler: " << e.what() << '\n';
1006+
resp.clear();
9571007
}
958-
});
959-
}
960-
inbound_pool_started_ = true;
1008+
WriteResponseOnSocket(pool->sock, protocol::GetResponseMsgID(mid), rid, resp);
1009+
pool->queued.fetch_sub(1);
1010+
}
1011+
});
9611012
}
9621013
}
963-
int workers = InboundWorkerCount();
964-
if (inbound_queued_.load() >= workers * 4) {
1014+
const int workers = InboundWorkerCount();
1015+
if (inbound_pool_->queued.load() >= workers * 4) {
9651016
// 队列满:立即回空响应,Agent 侧 failover 接管。
9661017
std::cerr << "[WARN] inbound queue full, fast-failing req_id=" << req_id << '\n';
967-
WriteResponseSilently(protocol::GetResponseMsgID(msg_id), req_id, {});
1018+
WriteResponseOnSocket(socket_, protocol::GetResponseMsgID(msg_id), req_id, {});
9681019
return;
9691020
}
970-
inbound_queued_.fetch_add(1);
1021+
inbound_pool_->queued.fetch_add(1);
9711022
{
972-
std::lock_guard<std::mutex> lock(inbound_pool_mutex_);
973-
inbound_queue_.emplace(msg_id, req_id, std::move(body));
1023+
std::lock_guard<std::mutex> lock(inbound_pool_->mu);
1024+
inbound_pool_->queue.emplace(msg_id, req_id, std::move(body));
9741025
}
975-
inbound_cv_.notify_one();
1026+
inbound_pool_->cv.notify_one();
9761027
}
9771028

978-
void TCPTransport::WriteResponseSilently(uint32_t resp_msg_id, uint32_t req_id, const std::vector<uint8_t>& body) {
1029+
// WriteResponseOnSocket 向指定 fd 尽力写一帧(fd 已关闭时仅返回 EBADF,
1030+
// 不影响调用方)。静态函数:worker 持 pool->sock 值,析构后仍安全。
1031+
void TCPTransport::WriteResponseOnSocket(socket_t sock, uint32_t resp_msg_id, uint32_t req_id,
1032+
const std::vector<uint8_t>& body) {
1033+
if (sock == INVALID_SOCKET_VALUE) {
1034+
return;
1035+
}
9791036
try {
9801037
auto frame = protocol::NewMessage(resp_msg_id, req_id, body);
9811038
std::vector<uint8_t> wrapped(4 + frame.size());
@@ -984,13 +1041,14 @@ void TCPTransport::WriteResponseSilently(uint32_t resp_msg_id, uint32_t req_id,
9841041
wrapped[2] = static_cast<uint8_t>((frame.size() >> 8) & 0xFF);
9851042
wrapped[3] = static_cast<uint8_t>(frame.size() & 0xFF);
9861043
std::memcpy(wrapped.data() + 4, frame.data(), frame.size());
987-
ssize_t sent = send(socket_, reinterpret_cast<const char*>(wrapped.data()),
988-
static_cast<int>(wrapped.size()), 0);
989-
(void)sent;
1044+
(void)send(sock, reinterpret_cast<const char*>(wrapped.data()),
1045+
static_cast<int>(wrapped.size()), 0);
9901046
} catch (...) {
9911047
// best-effort
9921048
}
9931049
}
9941050

1051+
1052+
9951053
} // namespace sdk
9961054
} // namespace croupier

0 commit comments

Comments
 (0)