fix: defer DSysInfo queries in plugins to avoid dlopen deadlock#3341
Merged
deepin-bot[bot] merged 1 commit intoJul 9, 2026
Merged
Conversation
Reviewer's GuideThis PR refactors DSysInfo-based OS/edition detection in several control-center plugins to use lazily initialized helper functions instead of namespace-scope/static globals, deferring any DSysInfo queries until first runtime use to avoid loader-lock-related deadlocks during plugin dlopen, and updates SPDX copyright year ranges accordingly. Sequence diagram for deferred DSysInfo queries in pluginssequenceDiagram
actor AppThread
participant QPluginLoader
participant Plugin
participant DSysInfo
participant dtklog
alt Before_change_global_const_init
AppThread->>QPluginLoader: load
QPluginLoader->>Plugin: run_namespace_globals
Plugin->>DSysInfo: uosEditionType
DSysInfo->>DSysInfo: ensureOsVersion
DSysInfo->>dtklog: qWarning
dtklog->>dtklog: acquire_write_lock
else After_change_lazy_static_helpers
AppThread->>QPluginLoader: load
QPluginLoader->>Plugin: construct_without_DSysInfo
QPluginLoader-->>AppThread: load_finished
AppThread->>Plugin: call_isCommunitySystem
Plugin->>Plugin: isCommunitySystem
Plugin->>DSysInfo: uosEditionType
DSysInfo->>DSysInfo: ensureOsVersion
DSysInfo->>dtklog: qWarning
dtklog->>dtklog: acquire_write_lock
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 1 issue, and left some high level feedback:
- The new
is*System()helpers are duplicated across multiple plugins (commoninfo,power,deepinid,sound); consider centralizing them in a shared header/source to avoid drift if the logic orDSysInfoAPI ever changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `is*System()` helpers are duplicated across multiple plugins (`commoninfo`, `power`, `deepinid`, `sound`); consider centralizing them in a shared header/source to avoid drift if the logic or `DSysInfo` API ever changes.
## Individual Comments
### Comment 1
<location path="src/plugin-commoninfo/operation/utils.h" line_range="34" />
<code_context>
-const bool IsEducationSystem = (DSysInfo::UosEducation == UosEdition); // 是否是教育版
-const bool IsDeepinDesktop = (DSysInfo::DeepinDesktop == DSysInfo::deepinType());//是否是Deepin桌面
-const bool IsNotDeepinUos = !DSysInfo::isDeepin(); // 是否是 Deepin/Uos 以外的发行版
+static inline bool isServerSystem()
+{
+ static const bool serverSystem = DSysInfo::UosServer == DSysInfo::uosType();
</code_context>
<issue_to_address>
**issue (complexity):** Consider using C++17 inline variables or a small helper function to cache system info once and avoid repetitive per-flag functions in this header.
You can keep the one-time caching semantics and avoid the per-flag function boilerplate by using C++17 inline variables (or at least reducing repetition with a helper).
### 1. Replace function+local-static with inline variables
If you’re building with C++17, this keeps semantics (single initialization, header-safe) while making the header shorter and more readable:
```cpp
inline const DSysInfo::UosType UosType = DSysInfo::uosType();
inline const DSysInfo::UosEdition UosEdition = DSysInfo::uosEditionType();
inline const bool IsServerSystem = (DSysInfo::UosServer == UosType);
inline const bool IsCommunitySystem = (DSysInfo::UosCommunity == UosEdition);
inline const bool IsProfessionalSystem= (DSysInfo::UosProfessional== UosEdition);
inline const bool IsHomeSystem = (DSysInfo::UosHome == UosEdition);
inline const bool IsEducationSystem = (DSysInfo::UosEducation == UosEdition);
inline const bool IsDeepinDesktop = (DSysInfo::DeepinDesktop == DSysInfo::deepinType());
inline const bool IsNotDeepinUos = !DSysInfo::isDeepin();
```
Call sites can still use the function-style naming if you prefer by adding trivial inline accessors:
```cpp
inline bool isServerSystem() { return IsServerSystem; }
inline bool isCommunitySystem() { return IsCommunitySystem; }
// etc.
```
This preserves the single-evaluation behavior while avoiding repeated `static inline bool ... { static const bool ...; return ...; }` boilerplate.
### 2. If inline variables aren’t available, at least deduplicate logic
You can keep your current “function + internal static” approach but reduce repetition:
```cpp
static inline bool isUosEdition(DSysInfo::UosEdition edition)
{
static const DSysInfo::UosEdition currentEdition = DSysInfo::uosEditionType();
return currentEdition == edition;
}
static inline bool isCommunitySystem() { return isUosEdition(DSysInfo::UosCommunity); }
static inline bool isProfessionalSystem(){ return isUosEdition(DSysInfo::UosProfessional); }
static inline bool isHomeSystem() { return isUosEdition(DSysInfo::UosHome); }
static inline bool isEducationSystem() { return isUosEdition(DSysInfo::UosEducation); }
```
This keeps your caching behavior but centralizes the edition query and removes duplicated initialization logic.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
yixinshark
force-pushed
the
defer-dsysinfo-plugin-load
branch
from
7b4e914 to
3165c9b
Compare
deepin pr auto review★ 总体评分：92分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // soundmodel.cpp 修复：统一使用 static inline 声明
// 同时建议抽取公共函数到共享头文件
// === 方案1：修复 soundmodel.cpp 的一致性问题 ===
// src/plugin-sound/operation/soundmodel.cpp
- static bool isServerSystem()
+ static inline bool isServerSystem()
{
static const bool serverSystem = Dtk::Core::DSysInfo::UosServer == Dtk::Core::DSysInfo::uosType();
return serverSystem;
}
// === 方案2（推荐）：抽取公共系统判断函数到独立共享头文件 ===
// src/shared/utils/systeminfo.h
#ifndef SYSTEMINFO_H
#define SYSTEMINFO_H
#include <DStandardPaths>
#include <DSysInfo>
DCORE_USE_NAMESPACE
namespace SystemInfo {
static inline bool isServerSystem()
{
static const bool serverSystem = DSysInfo::UosServer == DSysInfo::uosType();
return serverSystem;
}
static inline bool isCommunitySystem()
{
static const bool communitySystem = DSysInfo::UosCommunity == DSysInfo::uosEditionType();
return communitySystem;
}
static inline bool isProfessionalSystem()
{
static const bool professionalSystem = DSysInfo::UosProfessional == DSysInfo::uosEditionType();
return professionalSystem;
}
static inline bool isHomeSystem()
{
static const bool homeSystem = DSysInfo::UosHome == DSysInfo::uosEditionType();
return homeSystem;
}
static inline bool isEducationSystem()
{
static const bool educationSystem = DSysInfo::UosEducation == DSysInfo::uosEditionType();
return educationSystem;
}
static inline bool isDeepinDesktop()
{
static const bool deepinDesktop = DSysInfo::DeepinDesktop == DSysInfo::deepinType();
return deepinDesktop;
}
static inline bool isNotDeepinUos()
{
static const bool notDeepinUos = !DSysInfo::isDeepin();
return notDeepinUos;
}
} // namespace SystemInfo
#endif // SYSTEMINFO_H
// 各插件使用时只需 include 该头文件并调用 SystemInfo::isServerSystem() 等 |
caixr23
approved these changes
Jul 9, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, 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 |
The control center loads plugins via QPluginLoader during startup. Several
plugins called DSysInfo::uosType()/uosEditionType()/deepinType()/isDeepin()
in global/static initialization (namespace-scope const variables), which runs
while the dynamic loader holds the glibc/ld.so loader lock.
On systems where /etc/os-version is missing or malformed, DSysInfo internally
emits qWarning(), which enters the dtklog appender pipeline. Meanwhile another
thread already holding the dtklog write lock formats %{function} via
QRegularExpression/pcre2, which can trigger TLS/loader initialization and
wait on the same loader lock. This forms a classic deadlock:
Thread A: loader lock -> DSysInfo -> qWarning -> wait dtklog lock
Thread B: dtklog lock -> %{function} regex/pcre2/TLS -> wait loader lock
Replace the namespace-scope const globals (UosType, IsServerSystem,
IsCommunitySystem, etc.) with static inline lazy-init helper functions that
compute the value once on first runtime call via function-local statics.
This keeps the "compute once" semantics but moves the DSysInfo calls out of
the dlopen global constructor phase, breaking the loader-lock side of the
deadlock chain.
Affected plugins: sound, power, commoninfo, deepinid.
The helpers use `static inline` to preserve internal linkage (matching the
original const globals) and avoid ODR/symbol collisions across plugins that
share the same utils.h include guard name.
Log: defer DSysInfo queries in plugins to avoid dlopen deadlock
fix: 延迟插件中 DSysInfo 的调用以避免 dlopen 死锁
控制中心启动时通过 QPluginLoader 加载插件。多个插件在全局/静态初始化
（命名空间作用域 const 变量）中调用了 DSysInfo::uosType() 等接口，这些
代码在动态加载器持有 glibc/ld.so 加载锁期间执行。
当 /etc/os-version 缺失或格式异常时，DSysInfo 内部会触发 qWarning()，
进入 dtklog appender 写入流程。此时另一个线程已持有 dtklog 写锁，正在
格式化 %{function}，经由 QRegularExpression/pcre2 触发 TLS/加载器初始化，
并等待同一把加载锁，形成经典死锁：
线程 A：加载锁 -> DSysInfo -> qWarning -> 等待 dtklog 锁
线程 B：dtklog 锁 -> %{function} regex/pcre2/TLS -> 等待加载锁
将命名空间作用域的 const 全局变量（UosType、IsServerSystem 等）替换为
static inline 懒加载辅助函数，通过函数局部 static 在首次运行时调用时
计算一次。保留"只计算一次"语义，但把 DSysInfo 调用移出 dlopen 全局构造
阶段，打断死锁链中加载锁一侧。
涉及插件：sound、power、commoninfo、deepinid。
辅助函数使用 static inline 保持内部链接（与原 const 全局变量一致），
避免多个共用 utils.h include guard 名的插件之间产生 ODR/符号冲突。
Log: 延迟插件中 DSysInfo 的调用以避免 dlopen 死锁
Change-Id: I1b72cc0a72252730e640997b5481c96f3989343e
yixinshark
force-pushed
the
defer-dsysinfo-plugin-load
branch
from
3165c9b to
dc6e6ca
Compare
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Contributor
Author
BUG-368917 |
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
3 participants
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.
Summary
constglobals (UosType,IsServerSystem,IsCommunitySystem, etc.) in pluginutils.hheaders withstatic inlinelazy-init helper functions that compute the value once on first runtime call, movingDSysInfoqueries out of thedlopenglobal constructor phase.sound,power,commoninfo,deepinid.dlopen-> callsDSysInfo-> emitsqWarning-> enters dtklog and waits for the dtklog write lock; Thread B holds the dtklog write lock -> formats%{function}viaQRegularExpression/pcre2 -> triggers TLS/loader init -> waits for the loader lock.Background
On systems where
/etc/os-versionis missing or malformed,DSysInfo::ensureOsVersion()emitsqWarning(), which enters the dtklog appender pipeline. When this happens inside a plugin's global/static initialization (duringQPluginLoader::load()), the calling thread already holds the dynamic loader lock, creating a lock-order inversion with dtklog.The companion fixes in
dtkcore(DSysInfo::ensureOsVersionno longer logs while holdingDSysInfoPrivate::mutex) anddtklog(AbstractStringAppender::qCleanupFuncinfono longer usesQRegularExpressioninside the appender write lock) address the other two links in the chain. This PR addresses the control-center/application side.Test plan
sound,power,commoninfo,deepinid) without errors.dde-control-centeron a system with valid/etc/os-version— verify no regression in edition/type detection (server vs community vs professional, etc.).dde-control-centeron a system with missing/malformed/etc/os-version— verify no startup hang/deadlock.isServerSystem()/isCommunitySystem()return the same values as the oldIsServerSystem/IsCommunitySystemglobals on the same system.Summary by Sourcery
Defer system edition/type detection in control center plugins to avoid DSysInfo calls during plugin global initialization and break the loader/dtklog deadlock chain.
Bug Fixes:
Enhancements: