Repository navigation
fix: renew the SANE handle before scanning to fix the cloud scanner crash - #44
Merged
deepin-bot[bot] merged 1 commit intoSep 23, 2026
Conversation
re2zero
marked this pull request as draft
September 22, 2026 07:49
Reviewer's GuideThe PR fixes SANE handle corruption by removing cross-thread scanner enumeration from scan initiation, reopening devices safely in the worker before scans, and adding robust disconnect/error propagation with cached device-list updates for scanners and webcams. Sequence diagram for safe scanner scan initiationsequenceDiagram
actor User
participant ScannerDevice
participant ScannerWorker
participant SANE
User->>ScannerDevice: startCapture()
ScannerDevice->>ScannerWorker: doStartScan()
ScannerWorker->>SANE: reopenDevice()
alt device reopened
ScannerWorker->>SANE: doSetScanMode()
ScannerWorker->>SANE: sane_start()
SANE-->>ScannerWorker: scan status
else device unavailable
ScannerWorker->>ScannerWorker: reportDeviceDisconnected()
ScannerWorker-->>ScannerDevice: deviceDisconnected()
ScannerDevice-->>User: errorOccurred()
end
State diagram for scanner availability and scan failure handlingstateDiagram-v2
[*] --> Initialized
Initialized --> Opening: doOpenDevice()
Opening --> Connected: deviceOpened
Opening --> Initialized: onWorkerError()
Connected --> Scanning: startCapture / doStartScan()
Scanning --> Connected: captureCompleted
Scanning --> Connected: reportScanFailure() / reopenDevice()
Scanning --> Initialized: reportDeviceDisconnected()
Connected --> Initialized: deviceDisconnected
Initialized --> [*]
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/device/scannerdevice.cpp" line_range="142-152" />
<code_context>
- // Verify the device is still physically connected before scanning.
- // Without this check, a scanner that was unplugged after opening would
- // still allow scan attempts on a stale SANE handle.
- QStringList availableDevices = getAvailableDevices();
- // An empty list may indicate a transient SANE enumeration failure rather
- // than a real disconnection. Only treat it as a disconnect when the
- // enumeration succeeded (non-empty list) but our device is missing.
- if (!availableDevices.isEmpty() && !availableDevices.contains(m_currentDeviceName)) {
- qCWarning(app) << "Device" << m_currentDeviceName << "is no longer available.";
- m_deviceOpen = false;
- closeDevice();
- emit errorOccurred(tr("Scanner has been disconnected"));
- return;
- }
-
if (m_isCapturing) {
</code_context>
<issue_to_address>
**issue (bug_risk):** When a scanner is unplugged after opening and `sane_start()` returns an error, `doStartScan()` only emits the error and returns; it does not close the SANE handle or clear the worker/UI `m_deviceOpen` state. Removing this block therefore leaves the stale handle marked open, so later scan attempts continue using the disconnected handle instead of reopening or reporting the device as closed.
**Triggers:** When the device is physically disconnected after it was opened and a scan is attempted.
**Suggested fix:** On a failed `sane_start()`, close and clear the worker handle, emit the corresponding closed-state signal, and reset the UI-side `m_deviceOpen` state.
</issue_to_address>
re2zero
force-pushed
the
agent/pms-bug-bot/f019f714dfb9
branch
3 times, most recently
from
September 23, 2026 03:56
37386df to
db72096
Compare
re2zero
marked this pull request as ready for review
September 23, 2026 05:20
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/ui/scannerswidget.cpp" line_range="70-71" />
<code_context>
+ // Enumerating SANE devices while the worker is using an open handle can invalidate
+ // that handle (the backend may rebuild its device cache), so the known scanner list
+ // is kept until the device is idle again.
+ if (!scanner->isCapturing()) {
+ m_scannerNames = scanner->getAvailableDevices();
+ }
+ m_webcamNames = webcam->getAvailableDevices();
</code_context>
<issue_to_address>
**issue (bug_risk):** The scanner enumeration is still performed on the UI thread whenever `isCapturing()` is false, even though a completed scan leaves the SANE handle open in the worker. After `onCaptureCompleted()` sets `m_isCapturing` to false, refreshing the device list calls `getAvailableDevices()` concurrently with the worker-owned open handle and can again invalidate the handle or corrupt SANE backend state.
**Triggers:** When a scan has completed and the user refreshes the device list or otherwise triggers `updateDeviceList()` before the scanner is closed.
**Suggested fix:** Close or otherwise synchronize the worker-owned SANE handle before enumerating devices, or move scanner enumeration into the worker thread and serialize it with scan operations.
</issue_to_address>
…rash Re-opening the device before every scan is both the availability check and the only way to be sure the handle is usable: enumerating devices rebuilds the device cache of some backends (the deepin cloud scan backend does) and thereby invalidates every handle obtained before the enumeration. The check and the re-open now run in the worker thread instead of enumerating devices on the UI thread, so no stale handle is dereferenced when the scan starts. For the same reason the SANE device list is not re-enumerated while a scan is running or an open request is in flight, and the scanner open request is issued on the UI thread (it only queues work for the worker), so the scanner state is not written from two threads. A device that is known to be unusable is dropped from the device list and the user is told about it, instead of leaving a clickable dead entry behind: the scanner reports the device when a scan proves it is gone, the camera reports it when its video stream breaks, the camera entries are re-enumerated (a local /dev scan) when the device list is shown again, and an entry whose open fails is removed. A broken camera stream is reported only once instead of on every preview tick. The copyright year of the touched files is updated to the current year for the license check. 扫描前重开设备，既作为可用性判断，也保证句柄未被设备枚举失效（deepin 云扫描后端在枚举时会 重建设备缓存，使此前取得的 SANE 句柄悬垂），从而消除扫描时的崩溃。设备校验与重开移入 worker 线程，不再在 UI 线程枚举设备；扫描进行中或打开请求在途时也不再枚举 SANE 设备，且 打开请求改在 UI 线程发起（它只是给 worker 排队），避免扫描仪状态被两个线程写。 设备确认不可用时从设备列表移除并提示用户，不再残留可点击的死条目：扫描仪由扫描失败判定， 相机由视频流中断判定，返回列表时用本地 /dev 枚举刷新相机条目，打开失败的条目直接移除。 相机视频流中断只上报一次，不再每个预览周期重复提示。同步更新改动文件的版权年份以通过 license check。 Log: 修复云扫描仪扫描时应用无响应并崩溃的问题，设备不可用时移除列表条目并提示一次 PMS: BUG-378147 Influence: 云扫描仪扫描不再因句柄失效崩溃；扫描仪/相机不可用时从设备列表移除并只提示一次，避免用户点击已失效的设备。
re2zero
force-pushed
the
agent/pms-bug-bot/f019f714dfb9
branch
from
September 23, 2026 05:37
db72096 to
e14e14f
Compare
re2zero
changed the title
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法正确，逻辑清晰，边界处理完善。建议在 reportScanFailure() 中对 sane_get_devices() 返回的 deviceList 添加空指针保护注释，虽然当前代码已通过 deviceList && deviceList[i] 判空处理 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 代码结构清晰，注释完整。建议将重复的关闭设备逻辑提取为 closeDeviceHandle() 私有方法，将摄像头断开报告逻辑提取为 reportWebcamDisconnected() 方法，提高代码可维护性 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 性能良好，资源使用合理。reopenDevice() 的开销相对于实际扫描操作可忽略不计，且是正确性保障的必要代价。mainwindow.cpp:210-216 中返回设备列表时仅刷新摄像头（本地操作）而不重新枚举 SANE 设备（可能阻塞数秒），设计合理 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 无安全风险，安全合规。本次提交显著提升了线程安全性：所有 SANE API 调用（sane_open、sane_close、sane_get_devices、sane_start 等）现在均在 worker 线程中执行，消除了 UI 线程与 worker 线程之间的竞态条件。资源管理完善，所有错误路径均正确释放 SANE 句柄 💡 改进建议代码示例// 建议提取关闭设备句柄的公共方法到 ScannerWorker
void ScannerWorker::closeDeviceHandle()
{
if (m_deviceOpen && m_device) {
sane_close(m_device);
}
m_device = nullptr;
m_deviceOpen = false;
}
// 然后在 reopenDevice()、reportScanFailure()、doCloseDevice() 中复用
bool ScannerWorker::reopenDevice()
{
closeDeviceHandle();
// ... reopen logic
}
// 摄像头断开报告提取为方法
void WebcamDevice::reportDisconnectIfNeeded()
{
if (!m_disconnectReported) {
m_disconnectReported = true;
emit errorOccurred(tr("Device has been disconnected"));
emit deviceUnavailable(m_currentDeviceName, tr("Device has been disconnected"));
}
}本报告由 AI 代码审查工具自动生成 |
lzwind
approved these changes
Sep 23, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, re2zero 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 |
Contributor
Author
|
/merge |
|
This pr cannot be merged! (status: unstable) |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: unstable) |
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.
Problem
Scanning with a cloud scanner crashed the app (SIGSEGV in
ScannerWorker::doSetScanMode, PMS BUG-378147). The cloud backend (libsane-drs) hands out a handle that points into its device cache, andsane_get_devices()clears and rebuilds that cache - so enumerating devices invalidates every handle obtained before the enumeration.ScannerDevice::startCapture()enumerated devices on the UI thread, which invalidated the open handle that the worker then dereferenced.Fix
reopenDevice()), which both proves the device is still there and guarantees a handle that no enumeration can have invalidated.reportScanFailure(): the handle is closed before the device enumeration (that enumeration would invalidate it), and the device is only reported as disconnected if it is no longer enumerated or cannot be re-opened.ScannerDevice::isOpening()), so a UI-thread enumeration cannot invalidate a handle the worker is using. The scanner open request is issued on the UI thread as well (it only queues work for the worker), so the scanner state is not written from two threads./devscan) when the device list is shown again, and an entry whose open fails is removed.Verification
Follow-ups (not in this PR)
DeviceBase::deviceOpened(bool)/DeviceStatus::Offline, and the unreachable cancel path.PMS: BUG-378147