fix: move connection selection before network enable #472
+16
−14
Conversation
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
The connection selection logic was moved to the beginning of enableDevice function to ensure it runs before enabling networking. This prevents potential race conditions where network connections might change after enabling the device. The connection selection algorithm now correctly updates maxTs variable when finding the most recent autoconnect connection. Log: Fixed connection selection timing issue during device activation Influence: 1. Test device activation with multiple available connections 2. Verify autoconnect functionality works correctly 3. Check that the most recent connection is properly selected 4. Test network activation with disabled networking 5. Verify connection selection when no autoconnect connections exist fix: 将连接选择逻辑移到网络启用之前 连接选择逻辑被移到 enableDevice 函数的开头，确保在网络启用之前运行。这可 以防止在启用设备后网络连接可能发生变化的潜在竞争条件。连接选择算法现在在 找到最新的自动连接连接时正确更新 maxTs 变量。 Log: 修复设备激活期间连接选择时机问题 Influence: 1. 测试具有多个可用连接时的设备激活 2. 验证自动连接功能正常工作 3. 检查是否正确选择了最新连接 4. 测试网络禁用状态下的网络激活 5. 验证没有自动连接连接时的连接选择 PMS: BUG-336935
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideMoves connection selection earlier in NetworkThread::enableDevice to avoid races with networking enablement and fixes the autoconnect timestamp comparison so the most recent connection is selected correctly. Sequence diagram for updated enableDevice connection selection timingsequenceDiagram
actor System
participant NetworkThread
participant Device
participant Connection
participant NetworkManager
System->>NetworkThread: enableDevice(device)
activate NetworkThread
Note over NetworkThread,Device: 1) Select autoconnect connection before enabling networking
NetworkThread->>Device: availableConnections()
Device-->>NetworkThread: connPaths
loop for each connPath in connPaths
NetworkThread->>Connection: connPath.settings()
Connection-->>NetworkThread: settings
alt settings.autoconnect is true
NetworkThread->>Connection: settings.timestamp()
Connection-->>NetworkThread: ts
alt ts is newer than maxTs or no selection yet
NetworkThread-->>NetworkThread: update maxTs and connPath0
end
else settings.autoconnect is false
NetworkThread-->>NetworkThread: skip connection
end
end
NetworkThread->>NetworkManager: isNetworkingEnabled()
NetworkManager-->>NetworkThread: enabled
alt enabled is false
NetworkThread->>NetworkManager: setNetworkingEnabled(true)
end
alt connPath0 is empty
NetworkThread-->>System: "/"
else
NetworkThread-->>System: connPath0
end
deactivate NetworkThread
Flow diagram for autoconnect connection selection in enableDeviceflowchart TD
A[Start enableDevice] --> B[Get connPaths from device.availableConnections]
B --> C[Initialize connPath0 as empty]
C --> D[Initialize maxTs as default QDateTime]
D --> E{More connPaths?}
E -->|No| F{Is connPath0 empty?}
F -->|Yes| G[Return /]
F -->|No| H[Return connPath0]
E -->|Yes| I[Take next connPath]
I --> J[Load settings from connPath.settings]
J --> K{settings.autoconnect?}
K -->|No| E
K -->|Yes| L[ts = settings.timestamp]
L --> M{ts > maxTs or connPath0 is empty?}
M -->|No| E
M -->|Yes| N[Set maxTs = ts and connPath0 = connPath.path]
N --> E
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review代码审查报告1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
5. 改进建议以下是改进后的代码片段： QString NetworkThread::enableDevice(NetworkManager::Device::Ptr device)
{
auto connPaths = device->availableConnections();
qCDebug(DSM()) << "available connections:" << connPaths;
QString selectedConnPath;
QDateTime latestTimestamp;
for (auto &&connPath : connPaths) {
auto settings = connPath->settings();
if (!settings || !settings->autoconnect()) {
continue;
}
QDateTime ts = settings->timestamp();
if (ts > latestTimestamp || selectedConnPath.isEmpty()) {
latestTimestamp = ts;
selectedConnPath = connPath->path();
}
}
bool enabled = NetworkManager::isNetworkingEnabled();
if (!enabled) {
NetworkManager::setNetworkingEnabled(true);
}
if (!device->isEnabled()) {
device->setEnabled(true);
}
return selectedConnPath.isEmpty() ? "/" : selectedConnPath;
}6. 总结本次修改主要是代码块的移动，逻辑上没有问题，但可以通过以下改进提升代码质量：
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- When picking the most recent autoconnect connection, consider basing the initialization/guard purely on
maxTs.isValid()(or!maxTs.isNull()) instead ofconnPath0.isEmpty()so the selection logic is clearly tied to timestamp validity rather than the side effect of having chosen a path.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When picking the most recent autoconnect connection, consider basing the initialization/guard purely on `maxTs.isValid()` (or `!maxTs.isNull()`) instead of `connPath0.isEmpty()` so the selection logic is clearly tied to timestamp validity rather than the side effect of having chosen a path.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The connection selection logic was moved to the beginning of enableDevice function to ensure it runs before enabling networking. This prevents potential race conditions where network connections might change after enabling the device. The connection selection algorithm now correctly updates maxTs variable when finding the most recent autoconnect connection.
Log: Fixed connection selection timing issue during device activation
Influence:
fix: 将连接选择逻辑移到网络启用之前
连接选择逻辑被移到 enableDevice 函数的开头，确保在网络启用之前运行。这可
以防止在启用设备后网络连接可能发生变化的潜在竞争条件。连接选择算法现在在
找到最新的自动连接连接时正确更新 maxTs 变量。
Log: 修复设备激活期间连接选择时机问题
Influence:
PMS: BUG-336935
Summary by Sourcery
Adjust device activation flow to select the appropriate autoconnect connection before enabling networking and correct the timestamp-based selection logic.
Bug Fixes: