Repository navigation
fix: preserve wallpaper FD imports and reuse valid cached images - #199
deepin-bot[bot] merged 1 commit into
Conversation
Publish FD imports atomically so repeated imports replace earlier images and failed reads leave published images intact. Validate cache keys, retry interrupted reads, and keep imported sources readable for fallback clients and queued sizes. Reuse complete, readable size caches before consuming the descriptor, regenerate missing entries, and synchronize worker shutdown. Add isolated QtTest coverage and exclude test sources from the production plugin. 原子发布 FD 导入图片,使重复导入正确替换旧图,读取失败时保留已发布图片。 校验缓存键并重试中断读取,保留可读原图供客户端回退和后续尺寸处理使用。 完整有效的尺寸缓存命中时不再读取描述符,补齐缺失缓存,并同步工作线程退出。 增加使用独立缓存目录的 QtTest 回归测试,并从生产插件构建中排除测试源码。 Log: preserve wallpaper FD imports and reuse valid cached images Pms: BUG-335593 Change-Id: I3bfccfee1e4d23ee2c64997fe8f5f81e7de8c697
Reviewer's GuideThe PR hardens FD imports with validated temporary-file copying and atomic publication, preserves imported sources for fallback and future scaling, uses complete on-disk cache hits without consuming descriptors while regenerating missing sizes, and adds deterministic worker shutdown plus focused unit-test coverage. Sequence diagram for atomic wallpaper FD import and cache reusesequenceDiagram
participant Client
participant Service as WallpaperCacheService
participant Cache as CachedWallpaper
participant FD as ImageFD
participant Disk as CacheDisk
Client->>Service: GetProcessedImagePathByFd(fd, imagePathMd5, sizeArray)
Service->>Cache: findCachedImagePaths(imagePathMd5, sizes)
Cache->>Disk: Validate every cached file
alt Complete valid cache
Cache-->>Service: cached paths
Service-->>Client: cached paths
else Cache miss or missing size
Service->>Service: saveImageFromFd(fd, imagePathMd5)
Service->>FD: read()
Service->>Disk: Write temporary import
Service->>Disk: QImageReader::imageFormat()
Service->>Disk: rename() to publish atomically
Service->>Cache: getCachedImagePaths(destinationPath, sizes, true)
Cache-->>Service: generated or reused paths
Service-->>Client: processed paths
end
State diagram for synchronized scaling worker shutdownstateDiagram-v2
[*] --> Waiting
Waiting --> Processing: tasks available
Waiting --> Stopped: m_stop
Processing --> Waiting: task complete
Processing --> Stopped: m_stop
Stopped --> [*]
Flow diagram for preserving imported wallpaper sourcesflowchart LR
A[FD import] --> B[Temporary private file]
B --> C[Validate image format]
C --> D[Atomic rename to MD5 format path]
D --> E[Preserved source cache entry]
E --> F[Fallback and queued size processing]
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="src/plugin-qt/wallpapercache/wallpapercacheservice.cpp" line_range="232-235" />
<code_context>
+ qWarning() << "Failed to detect imported image format";
+ return {};
+ }
+ const QString destinationPath = kWallpaperCacheDir + "/" + imagePathMd5 + "." + QString::fromLatin1(format);
+ // POSIX rename atomically replaces an earlier import, including a longer
+ // image. Existing readers retain their opened file until they finish.
+ if (::rename(QFile::encodeName(originFile.fileName()).constData(),
+ QFile::encodeName(destinationPath).constData()) != 0) {
+ qWarning() << "Failed to publish imported image:" << errno;
</code_context>
<issue_to_address>
**Old imports remain on disk**
When the same key is imported in a different image format, `rename` replaces only the destination with the current format suffix, leaving any earlier `<key>.<format>` source files untouched. Repeatedly changing formats for a key therefore accumulates obsolete imported images in the cache directory.
Before or after publishing the new import, remove obsolete source files for that key without deleting the newly published file.
</issue_to_address>
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: ['代码逻辑正确,ScaleImageThread::run()的竞态条件修复采用标准等待-检查模式,处理得当', 'findCachedImagePaths方法的文件验证逻辑完善,正确处理了文件不存在、不可读、零大小等边界情况', 'saveImageFromFd的EINTR重试和errno处理符合POSIX最佳实践'] 2. 代码质量 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: ['建议为findCachedImagePaths添加函数文档注释,说明其用于查找并验证缓存图片路径的行为', '单元测试覆盖全面,包含9个测试用例,覆盖了正常流程、边界条件和安全场景', 'CMakeLists.txt正确排除了测试目录,避免测试代码编译进主库'] 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: ['缓冲区从4KB增加到64KB,显著提升了文件I/O吞吐量', 'GetProcessedImagePathByFd中的缓存命中优化避免了不必要的FD读取和文件写入', '使用constFind替代contains+operator[],避免了重复哈希查找', 'GetProcessedImagePathByFdWithBlur在不需要模糊处理时直接委托,避免了不必要的处理'] 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: ['路径遍历防护:saveImageFromFd中添加了MD5格式验证(\A[0-9a-fA-F]{32}\z),防止通过构造imagePathMd5参数进行目录遍历攻击', '原子文件操作:使用QTemporaryFile + POSIX rename()实现原子性文件替换,防止并发读取到部分写入的文件', '文件描述符验证:检查fd.fileDescriptor() < 0,防止无效FD处理', 'EINTR处理:正确处理被信号中断的系统调用,进行重试', '文件有效性验证:findCachedImagePaths验证文件存在性、可读性和非零大小,防止返回失效缓存', '防御性编程充分,无安全漏洞'] 💡 改进建议代码示例// 暂无代码示例本报告由 AI 代码审查工具自动生成 |
|
/forcemerge |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: robertkill, yixinshark 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 |
|
This pr force merged! (status: blocked) |
Summary
Repeated FD imports could fail to replace existing images, while deleting imported sources after scaling could invalidate fallback paths and later queued sizes. Publish imports atomically, preserve the source, and return complete valid size caches before reading the FD. Missing cache files are regenerated, and worker shutdown is synchronized.
The scaling, center-crop and pixmix algorithms and D-Bus interfaces are unchanged. Imported source files now remain in the cache for fallback and subsequent processing, which can increase disk usage.
Pms: BUG-335593
Validation
git diff --checkpassed. The original test-machine plugin was restored and its hash verified.Validation covers service behavior and interfaces; lock-screen/login visuals and prolonged workloads were not exercised.
Summary by Sourcery
Fix wallpaper FD imports and cache handling to safely reuse valid images while preserving sources needed for fallback and deferred processing.
Bug Fixes:
Enhancements:
Build:
Tests: