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

ILocatorFilter issues

    XMLWordPrintable

Details

    • All

    Description

      There are cases when one instance of ILocatorFilter is used inside Locator object and the same instance is also used independently. Example: CppEditor::CppClassesFilter or ClangCodeModel::CppClassesFilter. Note, that both instances of classes are registered also using CppModelManager::setClassesFilter(), so they are freely accessible through CppModelManager::classesFilter() and used outside of Locator:

      1. quickfixes.cpp : 1998 - inside matchName() - here machesFor() is called from main thread, so it may concur with the simultaneously running matchFor() in Locator's separate thread.
      2. Similar case in ElementTasks::hasClassDefinition() and ElementTasks::openClassDefinition().

      The similar case is when ILocatorFilter::allLocatorFilters() is used:

      1. Inside CppModelManager::findUnusedFunctions() - here the matchFor() is run in yet another thread, parallel to possibly running matchFor() from Locator object.
      2. Inside LineEditField::setupCompletion() - the same remark.

      The above scenarios (about matchFor() being used from 2 threads concurrently) are unlikely, however, not impossible. The user should be really fast to be able to start search in locator and then quickly initate e.g. a quick fix while the locator is still producing results.

      The possible fix is to:

      1. Make it possible to create instances of the same ILocatorFilter subclasse on the fly without registering them on static g_locatorFilters list inside ilocatorfilter.cpp. In this way, when having 2 instances of the same ILocatorFilter subclass, we could run both concurrently.
      2. Make ILocatorFilter::allLocatorFilters() private so that filters that belong to Locator object are not publicly shared, so that concurrent run on the same instance isn't possible. Make also Locator::filters() private and make friends to LocatorFiltersFilter, LocatorSettingsWidget and LocatorWidget only. Otherwise it may be misused in other cases.
      3. Instead of registering a global instance of a filter with CppModelManager::setClassesFilter(), etc... register a method for creating a new instances of locator filter. Something like: void setClassesFilterCreator(std::function<ILocatorFilter *()>). Instances created by the registered creator should not be added to the Locator object (i.e. should not land on static g_locatorFilters list). Leave the lifetime of returned locator filter into caller's responsibility. Move the registration method from CppModelManager into ILocatorFilter (or any other class inside CorePlugin) so that it's easily accessible for all dependent plugins. Currently, when LineEditField::setupCompletion() from ProjectExplorer plugin tries to fetch "Classes" filter, the CppModelManager::classesFilter() method from CppEditor plugin isn't available because of colliding dependencies (CppEditor depends on ProjectExplorer, but not vice versa).

      BTW, the class name disambiguation CppEditor::CppClassesFilter vs ClangCodeModel::CppClassesFilter would be really, really helpful when grasping internals. There are more ILocatorFilter subclass names to be disambiguated.

      Attachments

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

        Activity

          People

            jkobus Jarek Kobus
            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