feat: Listen for SIGTERM and exit the application gracefully.#440
feat: Listen for SIGTERM and exit the application gracefully.#440deepin-bot[bot] merged 2 commits intolinuxdeepin:masterfrom
Conversation
DDE may kill the deepin-camera circumstances, the logging system would consider it. 监听 SIGTERM 信号,正常退出应用。 由于DDE在某些情况下会kill掉deepin-camera进程,同时埋点会认为deepin-camera出错,所以这里监听信号,正常退出。 v20 BUG 分支合一到v25主线 Task: https://pms.uniontech.com/task-view-383481.html
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds SIGTERM handling in main.cpp so the deepin-camera process can exit cleanly when killed by DDE, avoiding it being logged as a crash, by registering a simple signal handler at application startup. Sequence diagram for graceful SIGTERM handling in deepin_camerasequenceDiagram
actor User
participant DDE
participant deepin_camera_main as deepin_camera_main
participant LoggingSystem
User->>DDE: Trigger action that ends camera session
DDE->>deepin_camera_main: SIGTERM
deepin_camera_main->>deepin_camera_main: handleSignal(sig)
deepin_camera_main->>LoggingSystem: Record normal application exit
deepin_camera_main-->>DDE: Process exits with code 0
Flow diagram for deepin_camera main with SIGTERM registrationflowchart TD
A["Start main"] --> B["Log 'Starting deepin-camera application...' via qInfo"]
B --> C["Register SIGTERM handler using signal(SIGTERM, handleSignal)"]
C --> D["Run deepin-camera main event loop and application logic"]
D --> E["Normal shutdown path (user closes app or app finishes)"]
E --> F["Exit with code 0"]
%% Asynchronous SIGTERM path
C -->|"Asynchronous SIGTERM received"| G["handleSignal invoked with sig"]
G --> H["Log 'Received signal: sig' via qWarning"]
H --> F
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/main.cpp:125-128` </location>
<code_context>
+// 由于DDE在某些情况下会kill掉deepin-camera进程,同时埋点会认为deepin-camera出错,所以这里监听信号,正常退出
+// 添加信号处理函数
+static void handleSignal(int sig)
+{
+ qWarning() << "Received signal:" << sig;
+ exit(0);
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid calling non–async-signal-safe functions (like qWarning and exit) inside a signal handler.
In a signal handler, only async-signal-safe functions are allowed. `qWarning()` and `exit()` are not safe here and can cause undefined behavior or deadlocks if the signal interrupts code holding locks. Prefer a minimal handler (e.g., set an atomic flag or write to a pipe) and let the main thread handle logging/shutdown, or use `_exit()`/`std::_Exit()` if you must terminate immediately without Qt teardown.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Due to issues with the upstream guvcview project (such as stripes and crashes) when decoding resolutions above 3840x2160, we need to limit the available resolution to 3840x2160. 限制可用分辨率到3840x2160。 由于上游guvcview项目在解码3840x2160分辨率以上时会出现异常(出现条纹、崩溃等),所以我们需要限制可用分辨率到3840x2160。 Bug: https://pms.uniontech.com/bug-view-343291.html v20 BUG 分支合一到v25主线 Task: https://pms.uniontech.com/task-view-383481.html
1923970 to
3dd0433
Compare
deepin pr auto review我来对这个Git diff进行详细审查:
a) 分辨率限制相关: // 优点:
- 将分辨率限制的硬编码值统一移到了v4l2_formats.h中
- 添加了is_valid_resolution()函数来统一验证分辨率
- 注释更详细地说明了限制分辨率的原因
// 建议改进:
- 可以考虑将MAX_WIDTH_LIMIT和MAX_HEIGHT_LIMIT定义为结构体或类,便于统一管理
- 建议添加分辨率限制的单元测试b) 信号处理相关: // 优点:
- 添加了信号处理机制,提高了程序的健壮性
- 注释说明了添加信号处理的原因
// 建议改进:
- handleSignal函数可以添加更多信号的处理(如SIGINT)
- 可以考虑使用更现代的C++信号处理方式,如std::signal
- 建议在退出前添加资源清理代码
// 优点:
- 添加了对分辨率的有效性检查
- 使用信号处理机制防止异常退出
// 建议改进:
- 在handleSignal中添加日志记录,便于问题追踪
- 可以考虑添加对分辨率上限的动态配置机制
a) 对is_valid_resolution函数的改进: int is_valid_resolution(int width, int height)
{
// 添加参数有效性检查
if (width <= 0 || height <= 0) {
return 0;
}
// 检查是否在限制范围内
if (width > MAX_WIDTH_LIMIT || height > MAX_HEIGHT_LIMIT) {
return 0;
}
// 检查对齐要求
return (width % 16 == 0 && height % 8 == 0);
}b) 对信号处理的改进: static void handleSignal(int sig)
{
qWarning() << "Received signal:" << sig;
// 添加资源清理代码
cleanup_resources();
exit(0);
}
// 注册更多信号
signal(SIGTERM, handleSignal);
signal(SIGINT, handleSignal); // 处理Ctrl+C
总的来说,这些改动提高了代码的可维护性和健壮性,但仍有改进空间。建议在后续版本中逐步完善这些方面。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lichaofan2008, lzwind 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 |
|
/merge |
DDE may kill the deepin-camera circumstances, the logging system would consider it.
监听 SIGTERM 信号,正常退出应用。
由于DDE在某些情况下会kill掉deepin-camera进程,同时埋点会认为deepin-camera出错,所以这里监听信号,正常退出。
v20 BUG 分支合一到v25主线
Task: https://pms.uniontech.com/task-view-383481.html
Summary by Sourcery
Enhancements: