Repository navigation
fix: restore concurrent transfer check in rpc handler - #836
Draft
pppanghu77 wants to merge 1 commit into
Draft
pppanghu77 wants to merge 1 commit into
pppanghu77 wants to merge 1 commit into
Conversation
1. Root cause: checkTransCanConnect() was commented out in handlerpcservice.cpp, disabling concurrent transfer detection 2. Fix: uncomment the call to restore concurrent transfer checking, preventing connection competition when two apps transfer to same device 3. Impact: second concurrent transfer request is now rejected instead of causing network error on receiving device Log: fix concurrent transfer conflict between file transfer and cooperation Influence: 1. Test concurrent file transfer from two apps to same device 2. Test file transfer after network disconnection and reconnection 3. Verify normal single-app file transfer still works fix: 恢复 RPC 处理器中的并发传输检查 1. 根因：handlerpcservice.cpp 中 checkTransCanConnect() 被注释禁用， 并发传输检查失效 2. 方案：取消注释恢复 checkTransCanConnect() 调用，防止两个应用 同时向同一设备发起传输时连接竞争导致接收失败 3. 影响：第二个并发传输请求将被拒绝，而非导致接收端网络错误 Log: 修复文件投送与跨端协同同时投送文件到同一设备的接收错误 Influence: 1. 测试两个应用同时向同一设备投送文件 2. 测试断网后重新发起文件传输 3. 验证单一应用正常文件传输不受影响 PMS: BUG-250595
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pppanghu77 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 |
pppanghu77
marked this pull request as draft
October 29, 2026 19:12
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRe-enables connection-level concurrency control in Sequence diagram for concurrent transfer rejectionsequenceDiagram
participant TransferApp
participant HandleRpcService
participant Comshare
participant SharedRpcConnection
participant JobManager
TransferApp->>HandleRpcService: handleRemoteLogin(info)
HandleRpcService->>Comshare: checkTransCanConnect()
Comshare-->>HandleRpcService: false
HandleRpcService-->>TransferApp: result=false
Note over SharedRpcConnection,JobManager: No competing connection or cascading cancellation
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/plugins/daemon/core/service/rpc/handlerpcservice.cpp" line_range="141" />
<code_context>
}
}
- bool statu = true;//Comshare::instance()->checkTransCanConnect();
+ bool statu = Comshare::instance()->checkTransCanConnect();
if (!authOK || !statu) {
lores.result = false;
</code_context>
<issue_to_address>
**Rejected logins still connect**
When authentication succeeds while `checkTransCanConnect()` returns false, `handleRemoteLogin` rejects the RPC login but passes `authOK` alone to `handleRpcLogin`; that handler creates a sender, updates connection state, and reports success to the local frontend, so the caller sees a successful connection for a rejected login.
Use `authOK && statu` as the admission result passed to `handleRpcLogin` and use it to prevent rejected logins from establishing connection state.
</issue_to_address>| } | ||
| } | ||
| bool statu = true;//Comshare::instance()->checkTransCanConnect(); | ||
| bool statu = Comshare::instance()->checkTransCanConnect(); |
There was a problem hiding this comment.
🟠 High · Rejected logins still connect
When authentication succeeds while checkTransCanConnect() returns false, handleRemoteLogin rejects the RPC login but passes authOK alone to handleRpcLogin; that handler creates a sender, updates connection state, and reports success to the local frontend, so the caller sees a successful connection for a rejected login.
Use authOK && statu as the admission result passed to handleRpcLogin and use it to prevent rejected logins from establishing connection state.
Prompt for AI agents
In `src/plugins/daemon/core/service/rpc/handlerpcservice.cpp` at line 141:
**Rejected logins still connect**
When authentication succeeds while `checkTransCanConnect()` returns false, `handleRemoteLogin` rejects the RPC login but passes `authOK` alone to `handleRpcLogin`; that handler creates a sender, updates connection state, and reports success to the local frontend, so the caller sees a successful connection for a rejected login.
Use `authOK && statu` as the admission result passed to `handleRpcLogin` and use it to prevent rejected logins from establishing connection state.
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.
Root Cause Analysis
The concurrent transfer check
checkTransCanConnect()inhandlerpcservice.cpp:141was commented out (commitf19577d2, 2023-12-14), hardcodingstatutotrue. This disabled concurrent transfer detection, allowing file transfer (dde-cooperation-transfer) and cross-device cooperation (dde-cooperation) to simultaneously initiate transfers to the same device. Both transfers share the same RPC port (UNI_RPC_PORT_TRANS=51599) and the same IP-based executor (remoteservice.cpp:196-210), causing connection-level competition. When one transfer fails due to competition,offlineCancelcascades and clears all jobs (jobmanager.cpp:341-360), terminating the other transfer and showing a network error on the receiving device.Fix
Uncomment the
checkTransCanConnect()call athandlerpcservice.cpp:141, restoring concurrent transfer checking. The second concurrent transfer request is now rejected at the connection check stage instead of causing connection competition and cascading cancellation.Change Safety Assessment
Code Safety
f19577d2("fix: Unable to transfer files again after network disconnection"), which commented outcheckTransCanConnect(). This change reverts that comment behavior — need to verify_cur_statusresets correctly after network disconnection to avoid recurrence.handlerpcservice.cpp:141inhandleRemoteLogin); function signature unchanged, no new public interface.Business Impact Scope
handlerpcservice.cpp:141(handleRemoteLoginfunction)Verification Suggestion
_cur_statusresets correctly (regression off19577d2original issue)根因分析
handlerpcservice.cpp:141的并发传输检查checkTransCanConnect()被注释禁用（commitf19577d2, 2023-12-14），statu硬编码为true。这导致文件投送和跨端协同两个应用同时向同一设备发起传输时请求均被接受。两个传输共享同一 RPC 端口（UNI_RPC_PORT_TRANS=51599）和同一 IP 的 executor 连接（remoteservice.cpp:196-210），造成连接级竞争。其中一个传输因竞争失败时，offlineCancel级联清除所有任务（jobmanager.cpp:341-360），导致另一个传输也被终止，接收端显示网络错误。修复方案
取消
handlerpcservice.cpp:141的注释，恢复checkTransCanConnect()调用。第二个并发传输请求将在连接检查阶段被拒绝，而非导致连接竞争和级联取消。改动安全评估
代码安全评估
f19577d2（"fix: Unable to transfer files again after network disconnection"）注释掉checkTransCanConnect()的产物，本次改动恢复该调用——需验证断网后_cur_status正确重置以避免原问题复发handlerpcservice.cpp:141的handleRemoteLogin函数）；函数签名不变，无新增公开接口业务影响范围
handlerpcservice.cpp:141（handleRemoteLogin函数）验证建议
_cur_status正确重置（回归f19577d2原问题）PMS: BUG-250595
Summary by Sourcery
Bug Fixes: