Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent dialog crash caused by thread safety issues #2468

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

liyigang1
Copy link
Contributor

  1. Add thread assertion in DialogManager singleton:

    • Ensure DialogManager is only accessed from main thread
    • Add Q_ASSERT check for thread safety
  2. Fix rename file operation issues:

    • Make needCheck parameter explicit in LocalFileHandler::renameFile
    • Pass correct needCheck flag in different rename scenarios:
      • Set to false for cut operations
      • Set to true for direct rename operations
  3. Code style improvements:

    • Fix spacing in variable initialization
    • Improve code formatting for better readability

This fixes potential crashes when showing dialogs from non-main threads and improves rename operation reliability.

Log: fix crash

1. Add thread assertion in DialogManager singleton:
   - Ensure DialogManager is only accessed from main thread
   - Add Q_ASSERT check for thread safety

2. Fix rename file operation issues:
   - Make needCheck parameter explicit in LocalFileHandler::renameFile
   - Pass correct needCheck flag in different rename scenarios:
     * Set to false for cut operations
     * Set to true for direct rename operations

3. Code style improvements:
   - Fix spacing in variable initialization
   - Improve code formatting for better readability

This fixes potential crashes when showing dialogs from non-main threads and improves rename operation reliability.

Log: fix crash
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. localfilehandler.h文件中,renameFile函数的默认参数needCheck被移除,这可能会导致调用该函数时没有传递needCheck参数而引发错误。建议在函数声明中保留默认参数,或者在使用该函数的地方明确传递参数。

  2. dialogmanager.cpp文件中,添加了Q_ASSERT(qApp->thread() == QThread::currentThread());,这是一个好的实践,确保DialogManager实例在主线程中创建。但是,如果这个断言失败,程序会立即终止,这可能会导致用户体验不佳。建议添加错误处理机制,而不是直接终止程序。

  3. docutfilesworker.cpp文件中,trashInfoUrl = trashInfo(fromInfo);fileName = fileOriginName(trashInfoUrl);两行代码之间缺少空行,这违反了代码格式化规范。建议在两行代码之间添加一个空行,以提高代码的可读性。

  4. docutfilesworker.cpp文件中,workData->currentWriteSize += (fromSize > 0 ? fromSize : FileUtils::getMemoryPageSize());这行代码中的三元运算符? :可以简化为std::max(fromSize, FileUtils::getMemoryPageSize()),以提高代码的可读性。

  5. docutfilesworker.cpp文件中,bool skip{false};这行代码使用了C++11的列表初始化语法,但是false是一个字面量,而不是一个变量。建议使用bool skip = false;来初始化skip变量。

  6. docutfilesworker.cpp文件中,if (fromSize <= 0)这行代码中的条件判断可以简化为if (!fromSize),以提高代码的可读性。

  7. docutfilesworker.cpp文件中,if (localFileHandler)这行代码中的条件判断可以简化为if (localFileHandler != nullptr),以提高代码的可读性。

  8. docutfilesworker.cpp文件中,return localFileHandler->renameFile(sourceUrl, targetUrl, false);这行代码中传递了false作为needCheck参数,这与localfilehandler.h文件中renameFile函数的默认参数不一致。建议在调用renameFile函数时明确传递参数,以避免潜在的混淆。

  9. fileoperationseventreceiver.cpp文件中,ok = fileHandler.renameFile(oldUrl, newUrl, true);这行代码中传递了true作为needCheck参数,这与localfilehandler.h文件中renameFile函数的默认参数不一致。建议在调用renameFile函数时明确传递参数,以避免潜在的混淆。

以上是本次代码审查的改进意见,希望能够对您有所帮助。

Copy link

github-actions bot commented Dec 5, 2024

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "src/plugins/common/core/dfmplugin-fileoperations/fileoperationsevent/fileoperationseventreceiver.cpp": [
        {
            "line": "            key = \"Name\";",
            "line_number": 364,
            "rule": "S106",
            "reason": "Var naming | ff5dbee3ee"
        }
    ]
}

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs, liyigang1

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

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

@Johnson-zs
Copy link
Contributor

/merge

@deepin-bot deepin-bot bot merged commit 6a3b6ee into linuxdeepin:release/eagle Dec 6, 2024
20 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