Repository navigation
fix(index): drop same-device paths outside monitored roots before enqueue - #292
Merged
deepin-bot[bot] merged 1 commit intoOct 26, 2026
Merged
Conversation
…ueue The event dispatcher broadcasts every write on a device, so the userspace watcher received events for paths that share a device with a monitored root but sit outside it. On a split-disk install "/", "/ostree" and similar siblings live on the same device, which meant every ostree checkout burst was enqueued even though /ostree is blacklisted. The per-runtime blacklist could not help: FSEventCollector applies it in shouldTrackPath() after dequeue, so a burst can overflow the queue and drop genuine root events first. Those drops raise eventsLost, which schedules a compensating full-index UPDATE and can interrupt an in-flight task. resolveAndFilterFullPath() already resolves the absolute path, so enforce root membership there with isDescendantOfRoot() before the event reaches the queue. rootPaths comes from the configured indexing_paths, so paths a user adds to the whitelist (for example under /media) keep working. Guarded on a non-empty rootPaths so the filter degrades to the previous behaviour instead of dropping every event.
Reviewer's GuideAdds an early monitored-root boundary check to resolved filesystem events, preventing same-device paths outside configured roots from being enqueued while preserving whitelist behavior and empty-root compatibility. Sequence diagram for pre-enqueue monitored-root filteringsequenceDiagram
participant Dispatcher
participant Watcher as VfsMonitorFileSystemWatcher
participant Queue
participant Indexer
Dispatcher->>Watcher: resolveAndFilterFullPath(dev, path)
Watcher->>Watcher: excludePredicate(fullPath)
alt rootPaths is non-empty
Watcher->>Watcher: isDescendantOfRoot(fullPath, monitoredRoot)
alt path is outside monitored roots
Watcher-->>Dispatcher: discard path
else path matches a monitored root
Watcher->>Queue: enqueue fullPath
Queue->>Indexer: process event
end
else rootPaths is empty
Watcher->>Queue: enqueue fullPath
Queue->>Indexer: process event
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Johnson-zs 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 |
![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 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/index/fsmonitor/vfsmonitorwatcher.cpp" line_range="574-582" />
<code_context>
+ // which in turn triggers a compensating full-index UPDATE. Same-device
+ // aliases cannot be told apart later, so the membership test has to
+ // happen before enqueue.
+ if (!rootPaths.isEmpty()) {
+ const bool inMonitoredRoot = std::any_of(rootPaths.cbegin(), rootPaths.cend(),
+ [&fullPath](const QString &monitoredRoot) {
+ return fullPath == monitoredRoot
+ || isDescendantOfRoot(fullPath, monitoredRoot);
+ });
+ if (!inMonitoredRoot)
+ continue;
+ }
+
return fullPath;
</code_context>
<issue_to_address>
**Outside renames overflow the queue**
When a burst of RENAME_TO events resolves outside every configured root, `drainSocket()` enqueues events with an empty destination, and `flushPendingBatch()` counts them against the queue limit. The queue overflows, dropping genuine root events and triggering a compensating full-index update.
Skip enqueuing a RENAME_TO event when its resolved destination is empty.
</issue_to_address>
### Comment 2
<location path="src/index/fsmonitor/vfsmonitorwatcher.cpp" line_range="575" />
<code_context>
+ // aliases cannot be told apart later, so the membership test has to
+ // happen before enqueue.
+ if (!rootPaths.isEmpty()) {
+ const bool inMonitoredRoot = std::any_of(rootPaths.cbegin(), rootPaths.cend(),
+ [&fullPath](const QString &monitoredRoot) {
+ return fullPath == monitoredRoot
</code_context>
<issue_to_address>
**New variables violate naming rule**
`inMonitoredRoot` and `monitoredRoot` use camelCase, so these new local variables violate the required snake_case naming convention.
Rename the variables to `in_monitored_root` and `monitored_root`.
Also at `src/index/fsmonitor/vfsmonitorwatcher.cpp:576`.
</issue_to_address>
Comment on lines
+574
to
+582
| if (!rootPaths.isEmpty()) { | ||
| const bool inMonitoredRoot = std::any_of(rootPaths.cbegin(), rootPaths.cend(), | ||
| [&fullPath](const QString &monitoredRoot) { | ||
| return fullPath == monitoredRoot | ||
| || isDescendantOfRoot(fullPath, monitoredRoot); | ||
| }); | ||
| if (!inMonitoredRoot) | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🟡 Medium · Outside renames overflow the queue
When a burst of RENAME_TO events resolves outside every configured root, drainSocket() enqueues events with an empty destination, and flushPendingBatch() counts them against the queue limit. The queue overflows, dropping genuine root events and triggering a compensating full-index update.
Skip enqueuing a RENAME_TO event when its resolved destination is empty.
Prompt for AI agents
In `src/index/fsmonitor/vfsmonitorwatcher.cpp` at lines 574-582:
**Outside renames overflow the queue**
When a burst of RENAME_TO events resolves outside every configured root, `drainSocket()` enqueues events with an empty destination, and `flushPendingBatch()` counts them against the queue limit. The queue overflows, dropping genuine root events and triggering a compensating full-index update.
Skip enqueuing a RENAME_TO event when its resolved destination is empty.| // aliases cannot be told apart later, so the membership test has to | ||
| // happen before enqueue. | ||
| if (!rootPaths.isEmpty()) { | ||
| const bool inMonitoredRoot = std::any_of(rootPaths.cbegin(), rootPaths.cend(), |
There was a problem hiding this comment.
⚪ Low · New variables violate naming rule
inMonitoredRoot and monitoredRoot use camelCase, so these new local variables violate the required snake_case naming convention.
Rename the variables to in_monitored_root and monitored_root.
Also at src/index/fsmonitor/vfsmonitorwatcher.cpp:576.
Prompt for AI agents
In `src/index/fsmonitor/vfsmonitorwatcher.cpp` at line 575:
**New variables violate naming rule**
`inMonitoredRoot` and `monitoredRoot` use camelCase, so these new local variables violate the required snake_case naming convention.
Rename the variables to `in_monitored_root` and `monitored_root`.
Also at `src/index/fsmonitor/vfsmonitorwatcher.cpp:576`.From AGENTS.md L59:
- Variables: `snake_case` (e.g., `config_ptr`, `event_handler`)
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 语法正确，逻辑清晰 ✅ 通过 潜在问题： 建议： ['逻辑无明显缺陷，建议确认 isDescendantOfRoot 对路径规范化（如尾部斜杠、符号链接）的处理与 fullPath 解析口径一致，避免同设备别名误判。'] 2. 代码质量 ✅评价： 代码结构清晰，注释完整 ✅ 通过 潜在问题： 建议： ['可考虑将 rootPaths 为空时的语义（当前直接放行）在注释中补充说明，明确空列表表示不做根成员限制而非全部丢弃。'] 3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 潜在问题： 建议： ['若 rootPaths 规模未来显著增长，可考虑使用有序集合或前缀树加速成员判定；当前规模下无需优化。'] 4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过
安全漏洞详情： 建议： [] 💡 改进建议代码示例// 当前实现已满足修复目标，无需额外修改。建议保持如下结构：
// 1. excludePredicate 过滤（已有）
// 2. 同设备根成员关系过滤（本次新增）
// 3. return fullPath;
//
// 可选：在 rootPaths 为空时显式注释放行语义，避免后续维护误判。本报告由 AI 代码审查工具自动生成 |
Author
|
/forcemerge |
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.
The event dispatcher broadcasts every write on a device, so the userspace
watcher received events for paths that share a device with a monitored root
but sit outside it. On a split-disk install "/", "/ostree" and similar
siblings live on the same device, which meant every ostree checkout burst
was enqueued even though /ostree is blacklisted.
The per-runtime blacklist could not help: FSEventCollector applies it in
shouldTrackPath() after dequeue, so a burst can overflow the queue and drop
genuine root events first. Those drops raise eventsLost, which schedules a
compensating full-index UPDATE and can interrupt an in-flight task.
resolveAndFilterFullPath() already resolves the absolute path, so enforce
root membership there with isDescendantOfRoot() before the event reaches
the queue. rootPaths comes from the configured indexing_paths, so paths a
user adds to the whitelist (for example under /media) keep working.
Guarded on a non-empty rootPaths so the filter degrades to the previous
behaviour instead of dropping every event.
Summary by Sourcery
Bug Fixes: