fix: Refactor DConfig wrapper class generation for thread safety and … #531

Dami-star · 2026-01-07T08:13:00Z

…lifecycle management

Problem:

Single-threaded design with weak state machine (Invalid -> Succeed/Failed)
No proper handling of object destruction during initialization
Signal emissions in worker thread context (incorrect thread context)
Fragile destructor unable to handle all cleanup scenarios

Solution:

Introduce Data layer separation (TreelandUserConfigData + TreelandUserConfig)
- Clear separation between internal data management and public API
- Enables safer object lifecycle management
Enhance state machine (3-state -> 5-state model)
- Add Initializing and Destroyed states
- Use atomic CAS operations for thread-safe state transitions
- States: Invalid -> Initializing -> (Succeed | Failed | Destroyed)
Improve async initialization and cleanup
- Use QPointer for safe backref checks (prevent use-after-free)
- Support 4 destruction paths: normal/failed/quick/mid-initialization
- Atomic state transitions with proper signal emission guards
Separate thread responsibilities
- updateValue(): Worker thread reads config values
- updateProperty(): Main thread updates properties and emits signals
- Use QMetaObject::invokeMethod for correct thread context

Improvements:

Thread safety: Complete atomic operations coverage
Memory safety: QPointer guards prevent dangling pointers
Code clarity: Layered architecture with clear responsibilities
Backward compatibility: API unchanged

deepin-ci-robot · 2026-01-07T08:13:10Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Dami-star

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

Details

Needs approval from an approver in each of these files:

debian/deepin/OWNERS

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

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

Copilot

Pull request overview

This PR refactors the DConfig C++ wrapper class generation tool (dconfig2cpp) to improve thread safety and lifecycle management. The refactor introduces a data layer separation pattern, enhances the state machine from 3 states to 5 states, and implements atomic operations for thread-safe state transitions.

Key changes:

Introduces a Data class (TreelandUserConfigData) to separate internal data management from the public API (TreelandUserConfig)
Expands state machine from Invalid/Succeed/Failed to Invalid/Initializing/Succeed/Failed/Destroyed with atomic CAS operations
Separates thread responsibilities: updateValue() in worker thread, updateProperty() in main thread with QMetaObject::invokeMethod for proper thread context

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tools/dconfig2cpp/main.cpp

zccrs

还没有看完

tools/dconfig2cpp/main.cpp

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

tools/dconfig2cpp/main.cpp

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-14T06:05:25Z

tools/dconfig2cpp/main.cpp

+                m_data->deleteLater();
+            }
+        } else if (oldStatus == static_cast<int>(Data::Status::Failed) || oldStatus == static_cast<int>(Data::Status::Invalid)) {
+            m_data->deleteLater();


The destructor has a potential memory leak scenario. When oldStatus is Initializing, m_data is not deleted. This can occur if the destructor is called while initialization is still in progress on the worker thread. The code should handle the Initializing state by marking it as Destroyed and allowing the worker thread to clean up, or explicitly delete m_data.

Suggested change

m_data->deleteLater();

m_data->deleteLater();

} else if (oldStatus == static_cast<int>(Data::Status::Initializing)) {

m_data->deleteLater();

当 oldStatus 为 Initializing 时，我们不会在此处删除，若在此处删除会导致空指针。工作线程将检测到 Destroyed 状态并自行清理 m_data。

src/log/dconfig_org_deepin_dtk_preference.hpp

18202781743 · 2026-01-14T08:54:24Z

toolGenerate/dconfig2cpp/dconf-example_meta.hpp

+    Q_ASSERT(!m_config.loadRelaxed());
+    m_config.storeRelaxed(config);
+    if (testPropertySet(0)) config->setValue(QStringLiteral("array"), QVariant::fromValue(p_array));
+    else updateValue(QStringLiteral("array"), QVariant::fromValue(p_array));


之前生成的里面存在一个问题，当已经在initializeInConfigThread函数中执行时，这时候dconfig_dconf-example_meta被调用方被释放了，会存在访问config变量是野指针的问题，目前这里还是会遇到吧，重构的时候可以考虑这个bug，

帮忙用最新代码测试下，另外linuxdeepin/treeland#680 有详细的测试用例

测试不是很好复现，配置项比较多的还可能容易点儿，看代码还有点问题，
在initializeInConfigThread保存了config变量这个，但这个变量会跟随dconfig_dconf-example_meta一起析构，后面继续访问config，还是会存在野指针问题吧，

toolGenerate/dconfig2cpp/dconf-example_meta.hpp

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

deepin-ci-robot · 2026-01-14T14:27:04Z

deepin pr auto review

代码审查报告

整体评估

这是一份由 dconfig2cpp 工具自动生成的 DConfig 配置管理类代码。代码经过重构，引入了 Data 内部类来管理配置数据，改进了线程安全性和内存管理。整体代码质量良好，但仍有一些可以改进的地方。

主要改进点

1. 架构设计

优点：引入了 Data 内部类，将数据存储与接口分离，提高了代码的可维护性
优点：使用 QPointer 安全地管理跨线程的对象引用
优点：状态管理更加清晰，使用枚举类 Status 明确定义了对象的生命周期状态

2. 线程安全

优点：使用 QAtomicInteger 和 QAtomicPointer 确保多线程环境下的原子操作
优点：使用 testAndSetOrdered 进行原子状态转换
优点：通过 QPointer 和 QMetaObject::invokeMethod 安全地跨线程访问对象

3. 内存管理

优点：使用 deleteLater() 安全地延迟删除对象
优点：在析构函数中正确处理不同状态下的资源释放
优点：通过 QObject::connect 的上下文对象机制确保信号槽连接的安全

需要改进的地方

1. 代码质量

重复代码过多
- 问题：属性相关的 getter/setter 代码有大量重复模式
- 建议：可以考虑使用模板或宏来减少重复代码
命名一致性
- 问题：部分变量命名不够一致，如 m_data 和 m_config
- 建议：统一使用 m_ 前缀表示成员变量
注释不足
- 问题：复杂的线程同步逻辑缺少详细注释
- 建议：添加更多注释解释线程同步机制和状态转换逻辑

2. 性能优化

属性访问优化

// 当前实现
bool autoDisplayFeature() const { return m_data->p_autoDisplayFeature; }

// 建议：可以考虑添加缓存或直接访问
bool autoDisplayFeature() const { return m_data->p_autoDisplayFeature.loadRelaxed(); }

信号发射优化
- 问题：每次属性变化都会发射两个信号
- 建议：考虑合并信号或提供批量更新接口

字符串处理

// 当前实现
QStringLiteral("autoDisplayFeature")

// 建议：使用静态常量或枚举
static constexpr const char* AutoDisplayFeatureKey = "autoDisplayFeature";

3. 安全性改进

空指针检查

// 当前实现
if (auto config = m_data->m_config.loadRelaxed()) {
    // 使用 config
}

// 建议：添加更多检查
if (auto config = m_data->m_config.loadRelaxed()) {
    if (!config->isValid()) {
        qWarning() << "Config is invalid";
        return;
    }
    // 使用 config
}

类型安全

// 当前实现
bool nv = qvariant_cast<bool>(v);

// 建议：添加类型检查
if (!v.canConvert<bool>()) {
    qWarning() << "Invalid type conversion";
    return;
}
bool nv = v.toBool();

异常处理
- 问题：缺少对可能抛出异常的操作的处理
- 建议：添加 try-catch 块处理异常情况

4. 其他建议

添加单元测试
- 建议为关键功能添加单元测试，特别是线程同步和状态转换逻辑
性能监控
- 建议添加性能监控代码，跟踪属性访问和更新的频率
文档完善
- 建议添加更详细的类和方法文档，特别是线程安全相关的说明

具体代码示例

改进示例1：属性访问优化

// 当前实现
bool autoDisplayFeature() const { return m_data->p_autoDisplayFeature; }

// 改进后
bool autoDisplayFeature() const { 
    return m_data->p_autoDisplayFeature.loadRelaxed(); 
}

改进示例2：信号发射优化

// 当前实现
Q_EMIT autoDisplayFeatureChanged();
Q_EMIT valueChanged(QStringLiteral("autoDisplayFeature"), v);

// 改进后
Q_EMIT autoDisplayFeatureChanged();
// 仅在需要时发射通用信号
if (m_emitGenericSignals) {
    Q_EMIT valueChanged(QStringLiteral("autoDisplayFeature"), v);
}

改进示例3：添加类型安全检查

// 当前实现
bool nv = qvariant_cast<bool>(v);

// 改进后
bool nv = false;
if (v.canConvert<bool>()) {
    nv = v.toBool();
} else {
    qWarning() << "Invalid type for property" << key;
    return;
}

总结

这份代码整体质量良好，特别是在线程安全和内存管理方面做了很多改进。主要改进方向是减少重复代码、提高性能和增强安全性。建议按照上述建议逐步改进，同时保持代码的向后兼容性。

…lifecycle management Problem: - Single-threaded design with weak state machine (Invalid -> Succeed/Failed) - No proper handling of object destruction during initialization - Signal emissions in worker thread context (incorrect thread context) - Fragile destructor unable to handle all cleanup scenarios Solution: 1. Introduce Data layer separation (TreelandUserConfigData + TreelandUserConfig) - Clear separation between internal data management and public API - Enables safer object lifecycle management 2. Enhance state machine (3-state -> 5-state model) - Add Initializing and Destroyed states - Use atomic CAS operations for thread-safe state transitions - States: Invalid -> Initializing -> (Succeed | Failed | Destroyed) 3. Improve async initialization and cleanup - Use QPointer for safe backref checks (prevent use-after-free) - Support 4 destruction paths: normal/failed/quick/mid-initialization - Atomic state transitions with proper signal emission guards 4. Separate thread responsibilities - updateValue(): Worker thread reads config values - updateProperty(): Main thread updates properties and emits signals - Use QMetaObject::invokeMethod for correct thread context Improvements: - Thread safety: Complete atomic operations coverage - Memory safety: QPointer guards prevent dangling pointers - Code clarity: Layered architecture with clear responsibilities - Backward compatibility: API unchanged

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

zccrs · 2026-01-15T05:27:39Z

src/log/dconfig_org_deepin_dtk_preference.hpp

 class dconfig_org_deepin_dtk_preference : public QObject {
    Q_OBJECT
-
+public:


这个改动不应该存在

zccrs · 2026-01-15T05:36:35Z

src/log/dconfig_org_deepin_dtk_preference.hpp

+        int oldStatus = m_data->m_status.fetchAndStoreOrdered(static_cast<int>(Data::Status::Destroyed));
+
+        if (oldStatus == static_cast<int>(Data::Status::Succeeded)) {
+            if (auto config = m_data->m_config.loadRelaxed()) {


对 m_config 应该是个断言，如果状态为 Succeeded，不应该出现它为空的情况。

zccrs · 2026-01-15T05:37:36Z

src/log/dconfig_org_deepin_dtk_preference.hpp

    static dconfig_org_deepin_dtk_preference* createGenericByName(DTK_CORE_NAMESPACE::DConfigBackend *backend, const QString &name, const QString &subpath = {}, QObject *parent = nullptr, QThread *thread = DTK_CORE_NAMESPACE::DConfig::globalThread())
    { return new dconfig_org_deepin_dtk_preference(thread, backend, name, {}, subpath, true, parent); }
+
    ~dconfig_org_deepin_dtk_preference() {


这个析构里不要销毁 data 数据，只负责在Succeeded状态下释放config对象。

zccrs · 2026-01-15T05:38:52Z

src/log/dconfig_org_deepin_dtk_preference.hpp

-        QMetaObject::invokeMethod(worker, [=, this]() {
+
+        // Use QPointer to safely check parent existence in worker thread
+        QPointer<dconfig_org_deepin_dtk_preference> safeThis(this);


不需要这个对象，只需要用一个QPointer保存 Data 对象即可

zccrs · 2026-01-15T05:49:56Z

src/log/dconfig_org_deepin_dtk_preference.hpp

+            // Atomically transition from Invalid to Initializing
+            if (!data->m_status.testAndSetOrdered(static_cast<int>(Data::Status::Invalid),
+                                                  static_cast<int>(Data::Status::Initializing))) {
+                return;


返回之前把 data->deleteLater 掉

zccrs · 2026-01-15T06:16:05Z

src/log/dconfig_org_deepin_dtk_preference.hpp

+        explicit Data()
+            : QObject(nullptr) {}
+
+        inline void markPropertySet(const int index, bool on = true) {


为什么不保持相对顺序呢，应该把 initializeInConfigThread 放为第一个函数

markPropertySet 这些是在后面

zccrs · 2026-01-15T06:19:12Z

src/log/dconfig_org_deepin_dtk_preference.hpp

-        } else {
-            updateValue(QStringLiteral("underlineShortcut"), QVariant::fromValue(p_underlineShortcut));
+
+        inline void updateValue(const QString &key, const QVariant &fallback = QVariant()) {


这个为什么相对于原实现改了那么多？

zccrs · 2026-01-15T06:20:27Z

src/log/dconfig_org_deepin_dtk_preference.hpp

+                updateValue(QStringLiteral("underlineShortcut"), QVariant::fromValue(p_underlineShortcut));
+            }
+            // Transition from Initializing to Succeeded
+            if (!m_status.testAndSetOrdered(static_cast<int>(Status::Initializing), static_cast<int>(Status::Succeeded))) {


把状态的设置放到外面做（紧接着initializeInConfigThread调用之后），不在这个函数体里

zccrs · 2026-01-15T06:21:32Z

src/log/dconfig_org_deepin_dtk_preference.hpp

+            // Transition from Initializing to Succeeded
+            if (!m_status.testAndSetOrdered(static_cast<int>(Status::Initializing), static_cast<int>(Status::Succeeded))) {
+                // Validation failed, likely Destroyed
+                if (m_status.loadRelaxed() == static_cast<int>(Status::Destroyed)) {


？？？前面都已经把状态设置为 Succeeded了，这里再判断还有意义吗，应该一开始在 testAndSetOrdered 中就明确判断是否为 Destroyed 状态，是的话不仅要销毁 config, 还要销毁data对象。

zccrs · 2026-01-15T06:22:19Z

src/log/dconfig_org_deepin_dtk_preference.hpp

+
+            // Value changed connection
+            // We use context object 'data' to ensure safety.
+            QObject::connect(config, &DTK_CORE_NAMESPACE::DConfig::valueChanged, data, [data](const QString &key) {


这个信号链接应该放到 initializeInConfigThread 中进行。

zccrs · 2026-01-15T07:13:23Z

tools/dconfig2cpp/main.cpp

@@ -1 +1 @@
 // SPDX-FileCopyrightText: 2024 UnionTech Software Technology Co., Ltd.


日期要更新了

deepin-ci-robot added a commit to linuxdeepin/dtk6core that referenced this pull request Jan 7, 2026

sync: from linuxdeepin/dtkcore

05250a0

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

deepin-ci-robot mentioned this pull request Jan 7, 2026

sync: from linuxdeepin/dtkcore linuxdeepin/dtk6core#179

Open

Dami-star mentioned this pull request Jan 7, 2026

fix: Refactor DConfig wrapper class generation for thread safety and … linuxdeepin/dtk6core#178

Closed

18202781743 requested review from 18202781743 and zccrs January 7, 2026 08:34

wineee requested a review from Copilot January 7, 2026 09:35

Copilot started reviewing on behalf of wineee January 7, 2026 09:35 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

zccrs requested changes Jan 13, 2026

View reviewed changes

Dami-star force-pushed the dconfig branch from 1499445 to 93c217e Compare January 13, 2026 05:22

deepin-ci-robot added a commit to linuxdeepin/dtk6core that referenced this pull request Jan 13, 2026

sync: from linuxdeepin/dtkcore

4833e32

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

zccrs requested changes Jan 13, 2026

View reviewed changes

tools/dconfig2cpp/main.cpp Outdated Show resolved Hide resolved

tools/dconfig2cpp/main.cpp Outdated Show resolved Hide resolved

Dami-star force-pushed the dconfig branch from 93c217e to f917895 Compare January 13, 2026 07:10

deepin-ci-robot added a commit to linuxdeepin/dtk6core that referenced this pull request Jan 13, 2026

sync: from linuxdeepin/dtkcore

8366a1c

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

Dami-star requested a review from zccrs January 14, 2026 01:52

Dami-star force-pushed the dconfig branch from f917895 to 75f9a3e Compare January 14, 2026 05:03

deepin-ci-robot added a commit to linuxdeepin/dtk6core that referenced this pull request Jan 14, 2026

sync: from linuxdeepin/dtkcore

0d8c3d6

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

zccrs requested a review from Copilot January 14, 2026 06:03

Copilot started reviewing on behalf of zccrs January 14, 2026 06:03 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

zccrs requested changes Jan 14, 2026

View reviewed changes

18202781743 reviewed Jan 14, 2026

View reviewed changes

toolGenerate/dconfig2cpp/dconf-example_meta.hpp Outdated Show resolved Hide resolved

Dami-star force-pushed the dconfig branch from 75f9a3e to 8dd017a Compare January 14, 2026 14:25

deepin-ci-robot added a commit to linuxdeepin/dtk6core that referenced this pull request Jan 14, 2026

sync: from linuxdeepin/dtkcore

847e158

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

Dami-star force-pushed the dconfig branch from 8dd017a to 76fa243 Compare January 15, 2026 03:06

deepin-ci-robot added a commit to linuxdeepin/dtk6core that referenced this pull request Jan 15, 2026

sync: from linuxdeepin/dtkcore

81a530e

Synchronize source files from linuxdeepin/dtkcore. Source-pull-request: linuxdeepin/dtkcore#531

zccrs requested changes Jan 15, 2026

View reviewed changes

tools/dconfig2cpp/main.cpp

@@ -1 +1 @@

// SPDX-FileCopyrightText: 2024 UnionTech Software Technology Co., Ltd.

Copy link

Member

zccrs Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

日期要更新了

		@@ -1 +1 @@
		// SPDX-FileCopyrightText: 2024 UnionTech Software Technology Co., Ltd.

fix: Refactor DConfig wrapper class generation for thread safety and … #531

Are you sure you want to change the base?

fix: Refactor DConfig wrapper class generation for thread safety and … #531

Uh oh!

Conversation

Dami-star commented Jan 7, 2026

Uh oh!

deepin-ci-robot commented Jan 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zccrs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

18202781743 Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

18202781743 Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

deepin-ci-robot commented Jan 14, 2026

deepin pr auto review

代码审查报告

整体评估

主要改进点

1. 架构设计

2. 线程安全

3. 内存管理

需要改进的地方

1. 代码质量

2. 性能优化

3. 安全性改进

4. 其他建议

具体代码示例

改进示例1：属性访问优化

改进示例2：信号发射优化

改进示例3：添加类型安全检查

总结

Uh oh!

18202781743 Jan 14, 2026 •

edited

Loading

18202781743 Jan 15, 2026 •

edited

Loading