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

[Dami-star]

…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

[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.

[zccrs]

还没有看完

[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]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

---

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

[deepin-ci-robot]

deepin pr auto review

Git Diff 代码审查报告

总体评估

这次代码变更主要是对 dconfig2cpp 工具生成的配置文件进行了重大重构，主要改进包括：

1. 修复了拼写错误（isInitializeSucceed → isInitializeSucceeded）
2. 引入了 Data 类来封装配置数据，改善了线程安全性
3. 改进了对象生命周期管理，使用了原子操作和状态机
4. 增加了新的配置属性（如 colorMode 和 defaultColorMode）

详细审查

1. 语法逻辑

优点：

1. 拼写错误修正：将 isInitializeSucceed 改为 isInitializeSucceeded，同时保留了旧方法并标记为已废弃，这是良好的向后兼容实践。
2. 状态管理改进：引入了 Data::Status 枚举，明确定义了对象状态（Invalid, Initializing, Succeeded, Failed, Destroyed），使状态转换更清晰。
3. 线程安全增强：使用 QPointer 和原子操作 (testAndSetOrdered) 来管理跨线程访问，避免了潜在的竞态条件。

需要改进的地方：

1. lambda 捕获问题：在 dconfig_org_deepin_dtk_preference 类中，多处使用了 [safeData, config, value] 这样的 lambda 捕获，但没有检查 safeData 的有效性。建议在 lambda 内部添加有效性检查：

   QMetaObject::invokeMethod(this, [this, newValue, key, value]() {
       if (!m_userConfig) return;  // 添加此检查
       // ... 其他代码
   });

2. 构造函数参数顺序：构造函数参数顺序与之前的版本不一致，可能导致现有代码需要修改。虽然这不是语法错误，但可能影响兼容性。

2. 代码质量

优点：

1. 代码组织：将配置数据封装到 Data 类中，使主类结构更清晰，职责分离更好。
2. 错误处理：增加了对配置创建失败的处理，包括状态转换和资源清理。
3. 注释完善：添加了更多注释解释代码意图，特别是状态转换和资源管理部分。

需要改进的地方：

1. 重复代码：initializeInConfigThread 和 updateValue 方法中有大量重复代码，可以考虑使用模板或宏来减少重复。
2. 魔法数字：属性索引使用硬编码数字（如 markPropertySet(0)），可以考虑使用枚举或常量提高可读性。

3. 代码性能

优点：

1. 原子操作优化：使用 testAndSetOrdered 和 fetchAndStoreOrdered 等原子操作，减少了锁的使用，提高了并发性能。
2. 延迟删除：使用 deleteLater 而不是直接删除，避免了在错误的线程中删除对象。

需要改进的地方：

1. 频繁的原子操作：在 updateValue 方法中，每次属性更新都会调用 markPropertySet，这涉及原子操作。如果属性更新非常频繁，可能会成为性能瓶颈。可以考虑批量更新或使用更高效的同步机制。

2. 字符串比较：在 updateValue 方法中使用字符串比较来确定哪个属性更新，可以考虑使用哈希或枚举来提高查找效率。

4. 代码安全

优点：

1. 线程安全：通过状态机和原子操作确保对象在不同线程间安全访问。
2. 资源管理：确保在对象销毁时正确释放资源，避免内存泄漏。
3. 空指针检查：多处添加了空指针检查，提高了代码健壮性。

需要改进的地方：

1. lambda 中的指针安全：如前所述，lambda 中捕获的指针可能已经失效，需要添加有效性检查。

2. 断言使用：代码中使用了 Q_ASSERT，但在发布版本中这些断言会被移除。对于关键的安全检查，应该使用运行时检查而不是断言。

3. 事件处理：在 event 方法中处理 QEvent::ThreadChange 事件，但没有处理其他可能的事件类型，可能导致未定义行为。

建议的改进

1. 添加指针有效性检查：

   QMetaObject::invokeMethod(this, [this, newValue, key, value]() {
       if (!m_userConfig) return;  // 添加此检查
       // ... 其他代码
   });

2. 使用枚举代替魔法数字：

   enum PropertyIndex {
       AutoDisplayFeature = 0,
       ColorMode = 1,
       DefaultColorMode = 2,
       // ... 其他属性
   };

3. 优化属性查找：

   // 使用哈希表代替字符串比较
   QHash<QString, int> propertyIndexMap = {
       {"autoDisplayFeature", 0},
       {"colorMode", 1},
       // ... 其他属性
   };

4. 改进错误处理：

   // 使用运行时检查代替断言
   if (!config || !config->isValid()) {
       qWarning() << "Invalid config";
       // 处理错误
       return;
   }

结论

这次代码变更总体上是一个重大改进，显著提高了代码的线程安全性和可维护性。主要改进包括状态管理、资源管理和错误处理。然而，仍有一些地方可以进一步优化，特别是在性能和安全性方面。建议在合并前进行充分的测试，特别是多线程场景下的测试。

[zccrs]

/approve

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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