Uploaded image for project: 'Qt Creator'
  1. Qt Creator
  2. QTCREATORBUG-25678

Possible crash in ClangModelManagerSupport on shutdown (or on session switch)

    XMLWordPrintable

Details

    • Bug
    • Resolution: Done
    • P2: Important
    • Qt Creator 6.0.0-beta1
    • Qt Creator 5.0.0-beta1
    • Editors
    • None
    • 33108795d6b2dd1e91942efb3c1c27ad23342295 (qt-creator/qt-creator/master)

    Description

      To reproduce:
      1. Apply this patch https://codereview.qt-project.org/c/qt-creator/qt-creator/+/347735
      2. Ensure you have switched on (in settings) C++ | Code Model | Use clangd. If you had to turn in on, restart creator now.
      3. Load creator project into creator
      4. As soon as you see the "Shutdown now" printout, close creator (or switch the session). It should crash.

      The reason is that ClangModelManagerSupport::updateLanguageClient() installs the future watcher for the running task, but in case of shutdown it doesn't stop the running task.


      After looking into internals I believe that the current implementation of Internal::generateCompilationDB() is not thread safe. Details as follows:

      It takes a ProjectInfo as an argument and later it does:

      const FilePath buildDir = buildDirectory(*projectInfo.project());
      

      projectInfo.project() returns a QPointer<>, and this class is not documented as reentrant, so it probably can't be used in non-main thread.
      Besides, the returned pointer to project itself may be null now (or may be deleted in any point in time inside the main thread), so it also can't be used directly in non-main thread.

      I've tried to replace the original arguments set:

      GenerateCompilationDbResult generateCompilationDB(CppTools::ProjectInfo projectInfo,
                                                        CompilationDbPurpose purpose)
      
      

      with the following set instead:

      void generateCompilationDB(
              QFutureInterface<GenerateCompilationDbResult> &futureInterface,
              const FilePath buildDir,
              const QList<ProjectPart> &projectParts,
              CompilationDbPurpose purpose)
      

      so that all usages of ProjectExplorer::Project are eliminated from the non-main thread. However, it was not sufficient, since the ProjectPart contains a pointer to ProjectExplorer::Project, which is still used indirectly from generateCompilationDB():

      generateCompilationDB() ->
      createFileObject() ->
      createClangOptions() ->
      const FileOptionsBuilder fileOptions(filePath, projectPart); ->
      addDiagnosticOptions() ->
      ClangProjectSettings &projectSettings = getProjectSettings(m_projectPart.project);
      

      I think that the implementation of Internal::generateCompilationDB() requires some refactoring so that all direct usages of ProjectExplorer::Project are avoided for all code paths that start from Internal::generateCompilationDB().

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

              kandeler Christian Kandeler
              jkobus Jarek Kobus
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes