Skip to content

fix: avoid duplicate logging under systemd with journal#543

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
mhduiy:log
Feb 27, 2026
Merged

fix: avoid duplicate logging under systemd with journal#543
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
mhduiy:log

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Feb 25, 2026

  1. Updated debian/control to require libdtklog-dev and libdtk6log-dev versions greater than 6.7.33
  2. Added shouldSkipConsoleAppender() method to detect when running under systemd with stdout connected to journal
  3. Modified initConsoleAppender() to skip console appender initialization when journal appender is active under systemd
  4. Modified initJournalAppender() to unregister existing console appender when journal appender is initialized under systemd
  5. Added environment variable DTK_FORCE_CONSOLE_LOGGING to override the behavior if needed

The changes prevent duplicate log output when applications run under systemd with journal logging enabled, since systemd automatically redirects stdout to journal. This avoids having both console and journal appenders writing the same logs.

fix: 避免在 systemd 下使用 journal 时重复日志输出

  1. 更新 debian/control 以要求 libdtklog-dev 和 libdtk6log-dev 版本大 于 6.7.33
  2. 添加 shouldSkipConsoleAppender() 方法检测是否在 systemd 环境下运行且 标准输出已连接到 journal
  3. 修改 initConsoleAppender() 在 systemd 下且 journal appender 激活时跳 过控制台 appender 初始化
  4. 修改 initJournalAppender() 在 systemd 下初始化 journal appender 时注 销已存在的控制台 appender
  5. 添加环境变量 DTK_FORCE_CONSOLE_LOGGING 用于在需要时覆盖此行为

这些更改防止了在 systemd 环境下运行且启用 journal 日志记录时出现重复日
志输出,因为 systemd 会自动将标准输出重定向到 journal。这避免了控制台和
journal appender 同时写入相同日志的问题。

@mhduiy
Copy link
Contributor Author

mhduiy commented Feb 25, 2026

BLumia
BLumia previously approved these changes Feb 27, 2026
@BLumia
Copy link
Member

BLumia commented Feb 27, 2026

image

这个别忘了 @mhduiy

1. Updated debian/control to require libdtklog-dev and libdtk6log-dev
versions greater than 6.7.33
2. Added shouldSkipConsoleAppender() method to detect when running under
systemd with stdout connected to journal
3. Modified initConsoleAppender() to skip console appender
initialization when journal appender is active under systemd
4. Modified initJournalAppender() to unregister existing console
appender when journal appender is initialized under systemd
5. Added environment variable DTK_FORCE_CONSOLE_LOGGING to override the
behavior if needed

The changes prevent duplicate log output when applications run under
systemd with journal logging enabled, since systemd automatically
redirects stdout to journal. This avoids having both console and journal
appenders writing the same logs.

fix: 避免在 systemd 下使用 journal 时重复日志输出

1. 更新 debian/control 以要求 libdtklog-dev 和 libdtk6log-dev 版本大
于 6.7.33
2. 添加 shouldSkipConsoleAppender() 方法检测是否在 systemd 环境下运行且
标准输出已连接到 journal
3. 修改 initConsoleAppender() 在 systemd 下且 journal appender 激活时跳
过控制台 appender 初始化
4. 修改 initJournalAppender() 在 systemd 下初始化 journal appender 时注
销已存在的控制台 appender
5. 添加环境变量 DTK_FORCE_CONSOLE_LOGGING 用于在需要时覆盖此行为

这些更改防止了在 systemd 环境下运行且启用 journal 日志记录时出现重复日
志输出,因为 systemd 会自动将标准输出重定向到 journal。这避免了控制台和
journal appender 同时写入相同日志的问题。
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

这份代码变更主要涉及日志管理系统的改进,包括依赖更新、许可证调整以及核心日志逻辑的优化。以下是对语法逻辑、代码质量、代码性能和代码安全方面的详细审查意见:

1. 语法逻辑

  • .reuse/dep5 文件变更

    • 问题:删除了 src/log/* 的版权声明。
    • 分析:虽然 LogManager.cpp 中的 SPDX-FileCopyrightText 更新到了 2026 年,但在 .reuse/dep5 中完全移除该目录的声明可能会导致合规工具(如 REUSE)无法正确识别该目录下文件的许可证信息,除非该目录下的所有文件头部都包含完整的 REUSE 标识。
    • 建议:确认 src/log/ 目录下的所有文件(包括头文件和其他源文件)是否都已包含完整的 SPDX-FileCopyrightTextSPDX-License-Identifier。如果是,则此变更是合理的;否则,建议保留或更新 .reuse/dep5 中的条目,确保许可证合规性。
  • debian/control 文件变更

    • 逻辑:将 libdtklog-devlibdtk6log-dev 的依赖关系增加了版本限制 (>> 6.7.33)
    • 分析:这表明新代码依赖于 dtklog 库的 6.7.33 版本之后的新功能或修复。这是一个合乎逻辑的变更,确保了构建环境的一致性。
    • 建议:确保 6.7.33 是正确的最低版本号,且该版本确实已发布并包含所需的接口。
  • LogManager.cpp 逻辑变更

    • 新增 shouldSkipConsoleAppender 函数
      • 逻辑:该函数通过检查 JOURNAL_STREAM 环境变量和 stdout 的文件状态(fstat)来判断当前进程是否由 systemd 服务管理且标准输出连接到了 journald。
      • 分析:逻辑严密。它遵循了 systemd 的标准做法(通过比对 st_devst_ino),能够准确识别日志是否会被 journald 自动捕获。
      • 建议:无重大逻辑问题。
    • initConsoleAppenderinitJournalAppender 的修改
      • 逻辑:在初始化时,如果检测到运行在 systemd 下且启用了 JournalAppender,则跳过或注销 ConsoleAppender,以防止日志重复。
      • 分析:这是一个很好的防御性编程实践,解决了日志重复写入的问题。
      • 潜在风险:在 initJournalAppender 中注销并删除 m_consoleAppender 是安全的,因为 initJournalAppender 通常在 initConsoleAppender 之后调用(需确认构造函数或初始化顺序)。如果调用顺序不确定,或者 m_consoleAppender 被其他地方引用,可能会导致悬空指针。
      • 建议:确保 DLogManager 的生命周期管理中,这两个初始化函数的调用顺序是确定的,或者增加对 m_consoleAppender 是否存在的更严格检查(虽然代码中已检查 d->m_consoleAppender,但需确保没有其他裸指针指向它)。

2. 代码质量

  • 注释与可读性

    • 代码中添加了清晰的注释解释了为什么要跳过 ConsoleAppender("avoid duplicate logging"),这提高了代码的可维护性。
    • shouldSkipConsoleAppender 函数名语义清晰,准确表达了其功能。
  • 宏定义的使用 (#ifdef Q_OS_LINUX)

    • 在包含 <sys/stat.h><unistd.h> 时使用了 Q_OS_LINUX 宏,这是良好的跨平台编码习惯,防止了在非 Linux 平台(如 Windows)上编译失败。
  • 环境变量检查

    • 使用 qEnvironmentVariableIsSetqgetenv 是 Qt 推荐的做法,代码风格统一。

3. 代码性能

  • 系统调用开销

    • shouldSkipConsoleAppender 函数中调用了 fstat(STDOUT_FILENO, &st)。这是一个系统调用,会有轻微的开销。
    • 分析:由于此函数仅在初始化阶段(initConsoleAppenderinitJournalAppender)调用一次,而不是在每次日志输出时调用,因此对整体性能影响可以忽略不计。
    • 建议:无需优化,当前实现是合理的。
  • 字符串处理

    • journalStream.split(':') 和后续的转换操作涉及少量内存分配。
    • 分析:同样因为只在初始化时运行一次,性能影响极小。
    • 建议:无需优化。

4. 代码安全

  • 空指针解引用风险

    • initJournalAppender 中,代码检查了 if (d->shouldSkipConsoleAppender() && d->m_consoleAppender)。这很好,防止了对空指针的解引用。
    • 建议:保持这种检查习惯。
  • 环境变量解析的安全性

    • journalStream.split(':')toULongLong 的转换包含了错误处理(ok1, ok2),如果环境变量格式不正确,函数会安全地返回 false,不会导致崩溃。
    • 建议:当前的错误处理是健壮的。
  • 竞态条件

    • fstat(STDOUT_FILENO) 检查时,文件描述符的状态理论上可能在极短时间内改变,但在日志初始化阶段,stdout 的重定向状态通常是稳定的。在此场景下风险极低。

总结与改进建议

这份代码变更整体质量很高,主要解决了 systemd 环境下的日志重复问题,并更新了依赖和版权信息。

主要改进建议:

  1. 许可证合规性检查:请务必确认 src/log/ 目录下所有文件头部是否已包含完整的 REUSE 标识,以验证移除 .reuse/dep5 中条目的安全性。
  2. 依赖版本号确认:确认 6.7.33 确实是引入必要功能或修复的正确版本。
  3. 日志输出的时机
    • initConsoleAppender 中使用了 qDebug() << "..."
    • 注意:此时 ConsoleAppender 尚未注册(或者正准备跳过注册),这条 qDebug 输出可能会被默认的 Qt 处理器处理,或者如果 JournalAppender 已经注册,则会被发送到 journal。这通常是可以接受的,但需注意不要在日志系统完全初始化之前产生过多噪音,或者导致递归(虽然 qDebug 通常不会触发递归)。
  4. 代码一致性
    • LogManager.cpp 头部的版权年份更新为 2022 - 2026。这是一个较长的跨度。如果这是项目规划,没问题;如果只是随意设置,建议根据实际情况更新(例如到当前年份)。

示例代码优化(微小的健壮性增强):

虽然当前代码处理得不错,但在解析环境变量时,可以稍微增加一点对空部分的检查,尽管 split 通常能处理:

// 在 shouldSkipConsoleAppender 中
auto parts = journalStream.split(':');
// 确保分割后正好有两部分,且都不为空
if (parts.size() != 2 || parts[0].isEmpty() || parts[1].isEmpty())
    return false;

总体而言,这是一个逻辑清晰、安全且实用的改进。

@mhduiy mhduiy requested a review from BLumia February 27, 2026 05:06
@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mhduiy
Copy link
Contributor Author

mhduiy commented Feb 27, 2026

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Feb 27, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 79285f1 into linuxdeepin:master Feb 27, 2026
16 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants