Repository navigation
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: max-lvs 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 GuideThis change prevents a second polkit authorization dialog after kernel-log truncation by caching file paths from the first authorized lookup and reusing them across KERN refresh, pagination, and parsing flows; non-KERN log handling is unchanged. Sequence diagram for cached kernel log refresh after truncationsequenceDiagram
actor User
participant Backend as LogBackend
participant DBus as DLDBusHandler
participant Service as LogViewerService
participant Parser as ParseThreadKern
User->>Backend: truncate and refresh KERN logs
Backend->>Backend: loadSegementPage()
alt cached kernel paths available
Backend->>Parser: parse(LOG_FILTER_BASE with cachedFilePaths)
Parser->>Parser: handleKern()
Parser->>Parser: reuse cachedFilePaths
else first KERN load
Backend->>DBus: getFileInfo(kern, false)
DBus->>Service: getFileInfo()
Service-->>DBus: authorized file paths
DBus-->>Backend: file paths
Backend->>Parser: parse(LOG_FILTER_BASE with cachedFilePaths)
Parser->>Parser: handleKern()
end
Parser->>DBus: getLineCount(file)
DBus-->>Parser: line counts
Parser-->>User: refreshed kernel log list
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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="application/logbackend.cpp" line_range="1997-1999" />
<code_context>
}
m_type2Filter[m_flag].segementIndex = nSegementIndex;
+ if (m_flag == KERN && !m_kernFilePaths.isEmpty()) {
+ m_type2Filter[m_flag].cachedFilePaths = m_kernFilePaths;
+ }
parse(m_type2Filter[m_flag]);
</code_context>
<issue_to_address>
**issue (bug_risk):** The cache is never invalidated or refreshed after it becomes non-empty, while `getFileInfo("kern", false)` dynamically scans `/var/log` and returns the current rotated kernel-log files. After log rotation removes or replaces a cached file, pagination or refresh uses the stale paths; `ParseThreadKern::handleKern()` then finds a missing path and finishes without producing data, leaving the kernel list empty or stale.
**Triggers:** When kernel log rotation or another filesystem change occurs after the cache is populated.
**Suggested fix:** Invalidate or refresh `m_kernFilePaths` when kernel log files rotate or disappear, and fall back to `getFileInfo` when a cached path no longer exists.
</issue_to_address>| if (m_flag == KERN && !m_kernFilePaths.isEmpty()) { | ||
| m_type2Filter[m_flag].cachedFilePaths = m_kernFilePaths; | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): The cache is never invalidated or refreshed after it becomes non-empty, while getFileInfo("kern", false) dynamically scans /var/log and returns the current rotated kernel-log files. After log rotation removes or replaces a cached file, pagination or refresh uses the stale paths; ParseThreadKern::handleKern() then finds a missing path and finishes without producing data, leaving the kernel list empty or stale.
Triggers: When kernel log rotation or another filesystem change occurs after the cache is populated.
Suggested fix: Invalidate or refresh m_kernFilePaths when kernel log files rotate or disappear, and fall back to getFileInfo when a cached path no longer exists.
…ation Cache the kernel log file paths and total line count from the first getFileInfo call in LogBackend. On subsequent loads (e.g. refresh after truncate), reuse the cached values instead of calling getFileInfo again, which triggers a second polkit auth dialog with a different action ID. However, the cached m_kernTotalLineCount and m_kernFilePaths were never invalidated after truncation. When the kernel log is truncated and refreshed, m_kernTotalLineCount retains the pre-truncation line count, causing m_segementCount to be too high. Pagination then reuses the stale count to decide there are more pages, triggering empty-page cascade loading (each involving pkexec logViewerAuth process spawn and DBus calls), causing noticeable lag and incorrect page indicators. Fix: add LogBackend::invalidateKernCache() that resets m_kernTotalLineCount to -1 and clears m_kernFilePaths. Call it in the truncate-refresh flow via a new sigTruncateRefresh signal / slot_truncateRefresh slot, NOT in clearAllFilter() (normal refresh depends on m_kernFilePaths to skip getFileInfo auth). Changes: - Add cachedFilePaths field to LOG_FILTER_BASE struct (structdef.h) - Add m_kernFilePaths and m_kernTotalLineCount cache members to LogBackend (logbackend.h) - Add LogBackend::invalidateKernCache() to reset kern cache (logbackend.h/.cpp) - LogBackend::getNextSegementIndex: populate cache on first call, reuse on subsequent calls; fix const ref iteration (logbackend.cpp) - LogBackend::loadSegementPage: inject cached paths into filter for KERN type before parse() (logbackend.cpp) - ParseThreadKern::handleKern: use cachedFilePaths from filter when available, skip getFileInfo call (parsethreadkern.cpp) - Add sigTruncateRefresh signal to LogListView, emit after truncateFile (loglistview.h/.cpp) - Add slot_truncateRefresh to DisplayContent: invalidateKernCache then slot_refreshClicked (displaycontent.h/.cpp) - Connect sigTruncateRefresh in LogCollectorMain (logcollectormain.cpp) Log: avoid second auth dialog when refreshing kern log after truncate Bug: https://pms.uniontech.com/bug-view-254665.html
28c4d6d to
d8ad122
Compare
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 语法正确,逻辑清晰,边界处理完善。缓存机制设计合理,哨兵值-1正确标识未初始化状态;txtFilePaths为空时缓存保持未初始化,三元运算符正确返回0;信号槽连接匹配正确。 2. 代码质量 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 代码结构清晰,注释完整。invalidateKernCache()方法有详细的Doxygen文档说明设计约束(不可在clearAllFilter中调用的原因);成员变量m_kernFilePaths和m_kernTotalLineCount有清晰的内联注释;const引用迭代改进(for (const auto &file : txtFilePaths))避免了不必要的QString拷贝。 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 性能良好,资源使用合理。缓存机制显著减少了DBus调用(getFileInfo/getLineCount)和polkit鉴权进程开销;修复了空页级联加载导致的性能问题(每个空页都触发pkexec进程和DBus调用)。哨兵模式(-1)避免不必要的重复计算。 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: 无安全风险,安全合规。缓存仅存储文件路径和行数,不涉及敏感数据;DBus调用沿用既有安全模式;无用户输入处理变更,无注入风险。 💡 改进建议代码示例// 本次提交为高质量的Bug修复,代码设计合理,无需改进
// 缓存机制:首次调用populate,后续复用,截断时invalidate
void LogBackend::invalidateKernCache()
{
m_kernTotalLineCount = -1;
m_kernFilePaths.clear();
}本报告由 AI 代码审查工具自动生成 |
Root Cause Analysis
When a user clears (truncates) kernel logs, the refresh flow calls
ParseThreadKern::handleKern()which invokesgetFileInfo("kern", false)via DBus.getFileInforequirescom.deepin.pkexec.logViewerAuthpolkit authorization, while truncate uses a different action ID (com.deepin.pkexec.logViewerTruncate). Since polkitauth_admin_keepis maintained per action ID, the truncate authorization does not cover the read authorization. When thelogViewerAuthkeep has expired, a second auth dialog is triggered; the user does not expect it, and the silentparseFinished(CancelAuth)causes the refresh to fail — the log list shows stale data until the user switches modules and returns.Key evidence:
application/parsethread/parsethreadkern.cpp:44callsgetFileInfoon every refresh;logViewerService/logviewerservice.cpp:807gatesgetFileInfobehindcheckAuth(s_Action_View); the two polkit policy files use distinct action IDs.Fix Approach
Cache the kernel log file paths obtained from the first
getFileInfocall inLogBackend::m_kernFilePaths. On subsequent loads (e.g. refresh after truncate), reuse the cached paths instead of callinggetFileInfoagain, avoiding the second polkit auth dialog. Truncate only empties file content — the file paths remain valid, so reusing cached paths is safe.Change Safety Assessment
Code Safety
Business Impact Scope
getFileInfo(populating the cache); subsequent loads reuse cached paths.Verification Suggestion
根因分析
用户清除内核日志后,刷新流程中
ParseThreadKern::handleKern()调用getFileInfo("kern", false)走 DBus 到LogViewerService::getFileInfo,后者要求com.deepin.pkexec.logViewerAuth鉴权。而清除日志使用不同的 polkit action ID(com.deepin.pkexec.logViewerTruncate),polkitauth_admin_keep按动作独立保持,truncate 鉴权不覆盖读取鉴权。当logViewerAuth的 keep 已过期时,getFileInfo触发第二个鉴权对话框,用户不预期该对话框,parseFinished(CancelAuth)静默发出,刷新失败——日志列表显示旧数据,直到用户切换模块再返回才刷新。关键证据:
application/parsethread/parsethreadkern.cpp:44每次刷新都调用getFileInfo;logViewerService/logviewerservice.cpp:807中getFileInfo被checkAuth(s_Action_View)门控;两个 polkit policy 文件使用不同的 action ID。修复方案
在
LogBackend::m_kernFilePaths中缓存首次getFileInfo调用获取的内核日志文件路径。后续刷新(如 truncate 后)复用缓存路径,不再调用getFileInfo,从而避免触发二次 polkit 鉴权。truncate 仅截断文件内容不改变文件路径,复用缓存路径安全。改动安全评估
代码安全评估
业务影响范围
getFileInfo(填充缓存),后续加载复用缓存路径。验证建议
Log: avoid second auth dialog when refreshing kern log after truncate
Bug: https://pms.uniontech.com/bug-view-254665.html
Summary by Sourcery
Avoid repeated kernel-log authorization prompts by coordinating truncation refreshes with cache invalidation and cached path reuse.
Bug Fixes:
Enhancements: