Skip to content

Commit 7e0683d

Browse files
ut003640deepin-bot[bot]
authored andcommitted
fix: resolve network detection failure and ping-pong switching in multi-NIC scenario
Two issues are fixed: 1. UB in HttpManager::get(url, timeoutSec): pass std::string to variadic function curl_easy_setopt instead of const char*, causing all HTTP requests to fail with "URL using bad/illegal format or missing URL". This made StatusChecker always report Limited and triggered the ping-pong switching loop. 2. Race condition in StatusChecker::realStartCheck(): when the check blocks for DNS timeout (~5s),InternetChecker may switch the primary connection in another thread. The stale result (based on old route) would incorrectly trigger network switching, creating a ping-pong loop between NICs. Fix: (1) use urlStd.c_str() in curl_easy_setopt; (2) snapshot the primary connection UUID before check and discard result if it changed. PMS:BUG-351163
1 parent 3f589ff commit 7e0683d

5 files changed

Lines changed: 43 additions & 18 deletions

File tree

config/org.deepin.dde.network.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -307,23 +307,23 @@
307307
"visibility": "private"
308308
},
309309
"httpRequestTimeout":{
310-
"value": 15,
310+
"value": 10,
311311
"serial": 0,
312312
"flags":["global"],
313313
"name":"httpRequestTimeout",
314314
"name[zh_CN]":"HTTP请求超时时间",
315-
"description[zh_CN]":"HTTP请求超时时间(单位:秒),默认15秒",
315+
"description[zh_CN]":"HTTP请求超时时间(单位:秒),默认10秒",
316316
"description":"HTTP request timeout in seconds, default 15 seconds",
317317
"permissions":"readwrite",
318318
"visibility":"private"
319319
},
320320
"httpConnectTimeout":{
321-
"value": 10,
321+
"value": 8,
322322
"serial": 0,
323323
"flags":["global"],
324324
"name":"httpConnectTimeout",
325325
"name[zh_CN]":"HTTP连接超时时间",
326-
"description[zh_CN]":"HTTP连接超时时间(单位:秒),默认10秒",
326+
"description[zh_CN]":"HTTP连接超时时间(单位:秒),默认8秒",
327327
"description":"HTTP connection timeout in seconds, default 10 seconds",
328328
"permissions":"readwrite",
329329
"visibility":"private"

network-service-plugin/src/system/connectivitychecker.cpp

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -264,34 +264,57 @@ void StatusChecker::realStartCheck()
264264
NetworkManager::ActiveConnection::Ptr pConnection = NetworkManager::primaryConnection();
265265
if (!pConnection.isNull()) {
266266
m_primaryId = pConnection->connection()->uuid();
267+
m_checkStartPrimaryId = m_primaryId;
268+
} else {
269+
m_checkStartPrimaryId.clear();
267270
}
271+
int httpTimeout = SettingConfig::instance()->httpRequestTimeout();
268272
bool networkIsOk = false;
273+
network::service::Connectivity detectedConnectivity = m_connectivity;
274+
QString detectedPortalUrl;
269275
for (const QString &url : m_checkUrls) {
270276
network::service::HttpManager http;
271-
network::service::HttpReply *httpReply = http.get(url);
277+
network::service::HttpReply *httpReply = http.get(url, httpTimeout);
272278
if (m_isStop) {
273279
qCDebug(DSM) << "Stop check connectivity";
274280
break;
275281
}
282+
if (httpReply->isTimeout()) {
283+
qCWarning(DSM) << "check network time out, network is unavaibled ";
284+
detectedConnectivity = network::service::Connectivity::Limited;
285+
break;
286+
}
276287
int httpCode = httpReply->httpCode();
288+
QString portalUrl = httpReply->portal();
277289
if (httpCode == 0) {
278290
qCWarning(DSM) << "Nework is unreachabel:" << url << httpReply->errorMessage();
291+
detectedConnectivity = network::service::Connectivity::Limited;
279292
continue;
280293
}
281-
282-
QString portalUrl = httpReply->portal();
283294
networkIsOk = true;
284295
qCDebug(DSM) << "Http reply code:" << httpCode << ", portal url:" << portalUrl;
285-
if (portalUrl.isEmpty()) {
286-
// if the portal is empty, I think it ok
287-
setConnectivity(network::service::Connectivity::Full);
288-
} else {
289-
setConnectivity(network::service::Connectivity::Portal);
290-
}
291-
setPortalUrl(portalUrl);
296+
detectedPortalUrl = portalUrl;
297+
detectedConnectivity = portalUrl.isEmpty() ? network::service::Connectivity::Full : network::service::Connectivity::Portal;
292298
break;
293299
}
294-
if (!m_isStop && !networkIsOk) {
300+
if (m_isStop)
301+
return;
302+
303+
// 主连接在检查期间可能已被 InternetChecker 切换,此时检测结果基于旧路由,必须丢弃
304+
NetworkManager::ActiveConnection::Ptr currentPrimary = NetworkManager::primaryConnection();
305+
QString currentPrimaryId;
306+
if (!currentPrimary.isNull())
307+
currentPrimaryId = currentPrimary->connection()->uuid();
308+
if (m_checkStartPrimaryId != currentPrimaryId) {
309+
qCInfo(DSM) << "Primary connection changed during check, discarding stale result"
310+
<< "check:" << m_checkStartPrimaryId << "current:" << currentPrimaryId;
311+
return;
312+
}
313+
314+
if (networkIsOk) {
315+
setConnectivity(detectedConnectivity);
316+
setPortalUrl(detectedPortalUrl);
317+
} else {
295318
NetworkManager::Device::List devices = NetworkManager::networkInterfaces();
296319
int disconnectCount = 0;
297320
for (NetworkManager::Device::Ptr device : devices) {
@@ -301,13 +324,13 @@ void StatusChecker::realStartCheck()
301324
}
302325
}
303326
qCDebug(DSM) << "Network is unreachabel, disconnect count:" << disconnectCount;
304-
setPortalUrl(QString());
305327
if (disconnectCount == devices.size()) {
306328
// 如果所有的网络设备都断开了,就默认让其变为断开的状态
307329
setConnectivity(network::service::Connectivity::Noconnectivity);
308330
} else {
309331
setConnectivity(network::service::Connectivity::Limited);
310332
}
333+
setPortalUrl(QString());
311334
}
312335
}
313336

network-service-plugin/src/system/connectivitychecker.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ private slots:
113113
QStringList m_checkUrls;
114114
bool m_isStop;
115115
QString m_primaryId;
116+
QString m_checkStartPrimaryId;
116117
};
117118

118119
class NMConnectionvityChecker : public ConnectivityChecker

network-service-plugin/src/system/internetchecker.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,15 @@ bool InternetChecker::checkInternetAccessible(int timeoutSec, bool &timedOut) co
129129

130130
bool InternetChecker::checkInternetAccessibleWithRetry(int maxRetry) const
131131
{
132+
int httpTimeout = SettingConfig::instance()->httpRequestTimeout();
132133
for (int i = 0; i < maxRetry; ++i) {
133134
// 每次检测前等待1秒,让NM路由表和DHCP先稳定
134135
QThread::sleep(1);
135136
bool timedOut = false;
136137
// 首次使用20秒超时,避免在无网线路由器的环境中长时间阻塞
137138
QElapsedTimer timer;
138139
timer.start();
139-
if (checkInternetAccessible((i == 0) ? 20 : 0, timedOut)) {
140+
if (checkInternetAccessible((i == 0) ? httpTimeout : 0, timedOut)) {
140141
return true;
141142
}
142143
if (timedOut) {

network-service-plugin/src/utils/httpmanager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ HttpReply *HttpManager::get(const QString &url, int timeoutSec)
236236
}
237237

238238
std::string urlStd = url.toStdString();
239-
curl_easy_setopt(curl, CURLOPT_URL, urlStd);
239+
curl_easy_setopt(curl, CURLOPT_URL, urlStd.c_str());
240240
curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1L);
241241
curl_easy_setopt(curl, CURLOPT_HTTPGET, 1);
242242
curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1L);

0 commit comments

Comments
 (0)