Skip to content

Commit 38646d6

Browse files
committed
fix(session): keep RPC routing alive across reconnects
The server kept one session per peer address. When a reconnect registered a new session before the old one was torn down, the new entry was dropped and the old session's disconnect then erased the address entirely, so B -> A requests silently stopped being delivered. Keep every session, route to the newest, and skip routing cleanup while another session for the same peer remains. Also surface synchronous send failures and briefly retry RPCs that no endpoint could accept. Log: keep RPC routing alive across reconnects PMS: TASK-396845 Influence: B -> A requests keep being delivered when a peer reconnects.
1 parent 3947d06 commit 38646d6

11 files changed

Lines changed: 306 additions & 100 deletions

‎src/lib/common/manager/sessionmanager.cpp‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@
1414
#include <QDir>
1515
#include <QStandardPaths>
1616
#include <QCoreApplication>
17+
#include <QTimer>
1718
#include <QUuid>
1819

1920
namespace {
21+
constexpr int kRpcRetryCount = 3;
22+
constexpr int kRpcRetryDelayMs = 300;
2023
// Transfer services are generated as random UUIDs. Restricting the accepted
2124
// format keeps a peer from naming a reserved service such as "barrier" and
2225
// stopping that forward when the transfer is released.
@@ -368,7 +371,28 @@ void SessionManager::sendRpcRequest(const QString &ip, int type, const QString &
368371
DLOG << "sendRpcRequest " << request;
369372
#endif
370373
WLOG << "sendRpcRequest " << request;
371-
_session_worker->sendAsyncRequest(ip, request);
374+
if (!_session_worker->sendAsyncRequest(ip, request))
375+
scheduleRpcRetry(ip, request, kRpcRetryCount);
376+
}
377+
378+
void SessionManager::scheduleRpcRetry(const QString &ip, const proto::OriginMessage &request,
379+
int remaining)
380+
{
381+
if (remaining <= 0) {
382+
WLOG << "Giving up RPC after retries, target:" << ip.toStdString()
383+
<< "mask:" << request.mask;
384+
return;
385+
}
386+
387+
// No endpoint claimed the target, so nothing was sent. The session may be
388+
// reconnecting or its IP mapping may not be ready yet; retry briefly
389+
// instead of dropping the request silently.
390+
WLOG << "RPC target not connected, retry in" << kRpcRetryDelayMs
391+
<< "ms, target:" << ip.toStdString() << "mask:" << request.mask;
392+
QTimer::singleShot(kRpcRetryDelayMs, this, [this, ip, request, remaining] {
393+
if (_session_worker && !_session_worker->sendAsyncRequest(ip, request))
394+
scheduleRpcRetry(ip, request, remaining - 1);
395+
});
372396
}
373397

374398
void SessionManager::sendTunnelMessage(const QString &target, int mask, const QByteArray &payload)

‎src/lib/common/manager/sessionmanager.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
#include <functional>
1616

17+
namespace proto { struct OriginMessage; }
18+
1719
class SessionWorker;
1820
class TransferWorker;
1921
class FileSizeCounter;
@@ -85,6 +87,7 @@ public slots:
8587
std::shared_ptr<TransferWorker> createTransWorker(const QString &jobid);
8688
void releaseTransWorker(const QString &jobid);
8789
void releaseTunnelJob(const QString &jobid);
90+
void scheduleRpcRetry(const QString &target, const proto::OriginMessage &request, int remaining);
8891

8992
struct TunnelJob {
9093
QString serviceId;

‎src/lib/common/manager/sessionworker.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,8 @@ bool SessionWorker::sendAsyncRequest(const QString &target, const proto::OriginM
395395
return true;
396396
}
397397

398-
DLOG << "Failed to send async request to target:" << target.toStdString();
398+
WLOG << "Failed to send async request to target:" << target.toStdString()
399+
<< "mask:" << request.mask;
399400
return false;
400401
}
401402

@@ -586,12 +587,11 @@ bool SessionWorker::doAsyncRequest(T *endpoint, const std::string& target, const
586587
{
587588
if (endpoint && endpoint->hasConnected(target)) {
588589
DLOG << "doAsyncRequest to " << target;
589-
endpoint->asyncRequestWithHandler(target, request, [this](int32_t type, const std::string &response) {
590+
return endpoint->asyncRequestWithHandler(target, request, [this](int32_t type, const std::string &response) {
590591
DLOG << "doAsyncRequest callback: " << type << " " << response;
591592
QString res = QString::fromStdString(response);
592593
emit onRpcResult(type, res);
593594
});
594-
return true;
595595
}
596596
return false;
597597
}

‎src/lib/common/session/protoclient.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ void ProtoClient::DisconnectAndStop()
1515

1616
bool ProtoClient::hasConnected(const std::string &ip)
1717
{
18+
// A stale host is tolerated here: onSend returns 0 when the session is
19+
// not handshaked, and the caller then retries or falls back.
1820
return ip == connectedHost();
1921
}
2022

‎src/lib/common/session/protoendpoint.cpp‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,10 @@ proto::OriginMessage ProtoEndpoint::syncRequest(const std::string &target, const
3838
return proto::OriginMessage();
3939
}
4040

41-
void ProtoEndpoint::asyncRequestWithHandler(const std::string &target, const proto::OriginMessage &request, RpcHandler resultHandler)
41+
bool ProtoEndpoint::asyncRequestWithHandler(const std::string &target, const proto::OriginMessage &request, RpcHandler resultHandler)
4242
{
43-
asyncRequest(target, request); // 发送异步请求
43+
if (!asyncRequest(target, request)) // 发送异步请求
44+
return false;
4445

4546
int32_t type = request.mask; // 获取请求的类型
4647
// 启动一个新的线程来处理结果
@@ -51,10 +52,11 @@ void ProtoEndpoint::asyncRequestWithHandler(const std::string &target, const pro
5152

5253
// 在启动的新线程中处理结果,主线程可以继续执行其他任务
5354
handlerThread.detach();
55+
return true;
5456
}
5557

5658

57-
void ProtoEndpoint::asyncRequest(const std::string &target, const proto::OriginMessage &msg)
59+
bool ProtoEndpoint::asyncRequest(const std::string &target, const proto::OriginMessage &msg)
5860
{
5961
int32_t type = msg.mask;
6062
std::future<proto::OriginMessage> futureResponse;
@@ -69,8 +71,24 @@ void ProtoEndpoint::asyncRequest(const std::string &target, const proto::OriginM
6971
clearActiveTarget();
7072
}
7173

74+
// request() fails synchronously when the session cannot send (for
75+
// example it is not handshaked or the target lookup failed). Surface that
76+
// so the caller can retry instead of silently dropping the RPC.
77+
if (futureResponse.wait_for(std::chrono::milliseconds(0)) == std::future_status::ready) {
78+
try {
79+
const auto early = futureResponse.get();
80+
// Preserve a (theoretical) immediate response.
81+
std::promise<proto::OriginMessage> ready;
82+
ready.set_value(early);
83+
futureResponse = ready.get_future();
84+
} catch (const std::exception &) {
85+
return false;
86+
}
87+
}
88+
7289
std::lock_guard<std::mutex> locker(_lock);
7390
_responses_by_type.insert(std::make_pair(type, PendingResponse { std::move(futureResponse), msg.id }));
91+
return true;
7492
}
7593

7694
void ProtoEndpoint::sendMessage(const std::string &target, int32_t mask, const std::string &payload)

‎src/lib/common/session/protoendpoint.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ class ProtoEndpoint: public FBE::proto::FinalClient
2727
void sendDisRequest();
2828
proto::OriginMessage syncRequest(const std::string &target, const proto::OriginMessage &msg);
2929

30-
// async call request and with 3s timeout result callback
31-
void asyncRequestWithHandler(const std::string &target, const proto::OriginMessage &request, RpcHandler resultHandler);
30+
// async call request and with 3s timeout result callback. Returns false
31+
// when the request could not be handed to the session at all.
32+
bool asyncRequestWithHandler(const std::string &target, const proto::OriginMessage &request, RpcHandler resultHandler);
3233

3334
// Send a one-way frame to a specific peer. It does not create a request
3435
// future and must not be routed through the multicast fallback.
@@ -50,7 +51,7 @@ class ProtoEndpoint: public FBE::proto::FinalClient
5051
}
5152

5253
private:
53-
void asyncRequest(const std::string &target, const proto::OriginMessage &msg);
54+
bool asyncRequest(const std::string &target, const proto::OriginMessage &msg);
5455
std::string getResponse(int32_t type);
5556

5657
protected:

‎src/lib/common/session/protoserver.cpp‎

Lines changed: 81 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -146,37 +146,32 @@ class ProtoSession : public NetUtil::Asio::SSLSession, public FBE::proto::FinalC
146146

147147
bool ProtoServer::hasConnected(const std::string &ip)
148148
{
149-
// std::cout << "check hasConnected: " << ip << std::endl;
149+
bool found = false;
150150

151151
// Use consistent lock ordering to avoid deadlock: always acquire mapping_lock before session_lock
152-
std::shared_lock<std::shared_mutex> mapping_locker(_ipmapping_lock);
153-
std::shared_lock<std::shared_mutex> session_locker(_sessionids_lock);
154-
155-
// print all session ids
156-
// for (auto &session : _session_ids) {
157-
// std::cout << "session ip: " << session.first << " id: " << session.second << std::endl;
158-
// }
159-
160-
// First try to find by direct IP match
161-
auto direct_it = _session_ids.find(ip);
162-
if (direct_it != _session_ids.end()) {
163-
std::cout << "hasConnected: " << ip << " true (direct match)" << std::endl;
164-
return true;
165-
}
152+
{
153+
std::shared_lock<std::shared_mutex> mapping_locker(_ipmapping_lock);
154+
std::shared_lock<std::shared_mutex> session_locker(_sessionids_lock);
166155

167-
// If not found, try to find via IP mapping (for NAT/Router scenarios)
168-
// Check if this IP is a real IP that maps to a remote endpoint IP
169-
auto real_to_remote = _real_to_remote_ip.find(ip);
170-
if (real_to_remote != _real_to_remote_ip.end()) {
171-
auto session_it = _session_ids.find(real_to_remote->second);
172-
if (session_it != _session_ids.end()) {
173-
std::cout << "hasConnected: " << ip << " true (via IP mapping: " << real_to_remote->second << ")" << std::endl;
174-
return true;
156+
found = _session_ids.find(ip) != _session_ids.end();
157+
158+
// If not found, try via IP mapping (for NAT/Router scenarios)
159+
if (!found) {
160+
auto real_to_remote = _real_to_remote_ip.find(ip);
161+
if (real_to_remote != _real_to_remote_ip.end())
162+
found = _session_ids.find(real_to_remote->second) != _session_ids.end();
175163
}
176164
}
177165

178-
std::cout << "hasConnected: " << ip << " false" << std::endl;
179-
return false;
166+
if (!found) {
167+
std::cout << "hasConnected: " << ip << " false" << std::endl;
168+
return false;
169+
}
170+
171+
// A registered session may still be completing its TLS handshake; onSend
172+
// reports that as a send failure, which lets the caller retry.
173+
std::cout << "hasConnected: " << ip << " true" << std::endl;
174+
return true;
180175
}
181176

182177
bool ProtoServer::startHeartbeat()
@@ -315,8 +310,9 @@ void ProtoServer::onConnected(std::shared_ptr<NetUtil::Asio::SSLSession>& sessio
315310

316311
{
317312
std::unique_lock<std::shared_mutex> locker(_sessionids_lock);
318-
// std::cout << "Server onConnected insert session ip: " << addr << " id: " << session->id() << std::endl;
319-
_session_ids.insert(std::make_pair(addr, session->id()));
313+
// Keep every session: a reconnect can register a new session before
314+
// the old one is torn down. Routing always picks the newest one.
315+
_session_ids.emplace(addr, session->id());
320316
}
321317

322318
_callbacks->onStateChanged(RPC_CONNECTED, addr);
@@ -329,36 +325,47 @@ void ProtoServer::onDisconnected(std::shared_ptr<NetUtil::Asio::SSLSession>& ses
329325
auto search_uuid = session->id();
330326
std::string addr = "";
331327
std::string real_ip = "";
332-
328+
bool replaced = false;
329+
333330
// Use consistent lock ordering: mapping_lock before session_lock to avoid deadlock
334331
{
335332
std::unique_lock<std::shared_mutex> mapping_locker(_ipmapping_lock);
336333
std::unique_lock<std::shared_mutex> session_locker(_sessionids_lock);
337-
334+
338335
// Find and remove session
339336
auto it = std::find_if(_session_ids.begin(), _session_ids.end(), [search_uuid](const std::pair<std::string, BaseKit::UUID>& pair) {
340337
return pair.second == search_uuid;
341338
});
342339

343340
if (it != _session_ids.end()) {
344-
//std::cout << "find connected by uuid, ip:" << it->first << std::endl;
345341
addr = it->first;
346342
_session_ids.erase(it);
347-
348-
// Clean up IP mapping for this remote endpoint
349-
auto remote_to_real = _remote_to_real_ip.find(addr);
350-
if (remote_to_real != _remote_to_real_ip.end()) {
351-
real_ip = remote_to_real->second;
352-
_real_to_remote_ip.erase(real_ip);
353-
_remote_to_real_ip.erase(addr);
354-
std::cout << "Cleaned up IP mapping for disconnected session: " << real_ip << " -> " << addr << std::endl;
343+
344+
// A newer session for the same peer may already be connected.
345+
// Keep its routing state and do not report the peer as gone.
346+
replaced = _session_ids.find(addr) != _session_ids.end();
347+
348+
if (!replaced) {
349+
// Clean up IP mapping for this remote endpoint
350+
auto remote_to_real = _remote_to_real_ip.find(addr);
351+
if (remote_to_real != _remote_to_real_ip.end()) {
352+
real_ip = remote_to_real->second;
353+
_real_to_remote_ip.erase(real_ip);
354+
_remote_to_real_ip.erase(addr);
355+
std::cout << "Cleaned up IP mapping for disconnected session: " << real_ip << " -> " << addr << std::endl;
356+
}
355357
}
356358
} else {
357359
std::cout << "did not find connected id:" << search_uuid << std::endl;
358360
return;
359361
}
360362
}
361363

364+
if (replaced) {
365+
std::cout << "Session superseded, keep connection state for: " << addr << std::endl;
366+
return;
367+
}
368+
362369
_callbacks->onStateChanged(RPC_DISCONNECTED, addr);
363370
}
364371

@@ -372,40 +379,51 @@ size_t ProtoServer::onSend(const void *data, size_t size)
372379
return size;
373380
}
374381

375-
std::shared_ptr<NetUtil::Asio::SSLSession> session = nullptr;
376382
std::string target_ip = _active_target;
383+
_active_target.clear();
384+
385+
BaseKit::UUID sessionId;
386+
bool found = false;
377387

378-
// First try direct session lookup
388+
// Use consistent lock ordering: mapping_lock before session_lock. Prefer
389+
// the newest session when a reconnecting peer has both an old and a new
390+
// one registered.
379391
{
392+
std::shared_lock<std::shared_mutex> mapping_locker(_ipmapping_lock);
380393
std::shared_lock<std::shared_mutex> session_locker(_sessionids_lock);
381-
auto session_it = _session_ids.find(target_ip);
382-
if (session_it != _session_ids.end()) {
383-
session = FindSession(session_it->second);
384-
}
385-
}
386394

387-
// If not found, try via IP mapping (for NAT/Router scenarios)
388-
if (!session) {
389-
// Use consistent lock ordering: mapping_lock before session_lock
390-
std::shared_lock<std::shared_mutex> mapping_locker(_ipmapping_lock);
391-
auto real_to_remote = _real_to_remote_ip.find(target_ip);
392-
if (real_to_remote != _real_to_remote_ip.end()) {
393-
std::string remote_ip = real_to_remote->second;
394-
std::shared_lock<std::shared_mutex> session_locker(_sessionids_lock);
395-
auto session_it = _session_ids.find(remote_ip);
396-
if (session_it != _session_ids.end()) {
397-
session = FindSession(session_it->second);
398-
std::cout << "Found session via IP mapping: " << target_ip << " -> " << remote_ip << std::endl;
399-
}
395+
auto newestFor = [this](const std::string &addr, BaseKit::UUID *id) {
396+
auto range = _session_ids.equal_range(addr);
397+
if (range.first == range.second)
398+
return false;
399+
auto newest = range.second;
400+
--newest;
401+
*id = newest->second;
402+
return true;
403+
};
404+
405+
found = newestFor(target_ip, &sessionId);
406+
407+
// If not found, try via IP mapping (for NAT/Router scenarios)
408+
if (!found) {
409+
auto real_to_remote = _real_to_remote_ip.find(target_ip);
410+
if (real_to_remote != _real_to_remote_ip.end())
411+
found = newestFor(real_to_remote->second, &sessionId);
400412
}
401413
}
402414

403-
if (session) {
404-
session->SendAsync(data, size);
405-
} else {
415+
if (!found) {
406416
std::cout << "No session found for target: " << target_ip << std::endl;
417+
return 0;
418+
}
419+
420+
auto session = FindSession(sessionId);
421+
if (!session || !session->IsHandshaked()) {
422+
std::cout << "Session not ready for target: " << target_ip << std::endl;
423+
return 0;
407424
}
408425

409-
_active_target = "";
410-
return size;
426+
// Report failure instead of pretending the frame was delivered; the FBE
427+
// request then fails its future instead of hanging until timeout.
428+
return session->SendAsync(data, size) ? size : 0;
411429
}

‎src/lib/common/session/protoserver.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// SPDX-FileCopyrightText: 2023 UnionTech Software Technology Co., Ltd.
1+
// SPDX-FileCopyrightText: 2023 - 2026 UnionTech Software Technology Co., Ltd.
22
//
33
// SPDX-License-Identifier: GPL-3.0-or-later
44

@@ -40,9 +40,10 @@ class ProtoServer : public NetUtil::Asio::SSLServer, public ProtoEndpoint
4040
void onHeartbeatTimeout(bool canceled);
4141

4242
private:
43-
// <ip, sessionid>
43+
// <ip, sessionid>. A reconnecting peer can briefly have both the old and
44+
// the new session, so keep all of them and route to the newest one.
4445
std::shared_mutex _sessionids_lock;
45-
std::map<std::string, BaseKit::UUID> _session_ids;
46+
std::multimap<std::string, BaseKit::UUID> _session_ids;
4647

4748
// IP mapping for NAT/Router scenarios: <real_ip, remote_endpoint_ip>
4849
std::shared_mutex _ipmapping_lock;

0 commit comments

Comments
 (0)