Uploaded image for project: 'Forge'
  1. Forge
  2. FORGE-763

ShellImpl "noInitMode" concept mismanages completers

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Done
    • Icon: Major Major
    • 1.2.1.Final
    • 1.2.0.Final
    • UI - Shell
    • None

      The concept at work in the current code is that a number of completers are set up when the ShellImpl#init() Startup observer is called (that number currently happens to be 1--the instance of PluginCommandCompleter that is also passed to the Startup observer). These are added to an AggregateCompleter and stored as an instance variable of the ShellImpl. At the same time, if the shell is determined to be in noInitMode, these (1) completers are added to a list deemed _deferredCompleters. The idea is that in noInitMode the reader will not be known at init time. Unfortunately, there are a number of implementation problems:

      • the method that eventually accomplishes setting the completer on the reader, _initCompleters() is parameterized with a completer, but this completer is never used in the body of the method. Instead, the completer member is sent.
      • the _deferredCompleters list is only initialized if it is null. In the test scenario, the result of this is that the list grows by one repeated instance of the same PluginCommandCompleter for every @Test method executed in a given unit test class.
      • the noInitMode portion of initReadersAndStreams() calls _initCompleters() once per element of _deferredCompleters; this has two effects: it sets the completion handler on the reader multiple times (which is only wasteful), and it adds the instance-bound completer multiple times. As a result, when e.g. #promptWithCompleter() temporarily removes the shell's completer, there are still n remaining copies of the same instance hanging around.

      If the two identified oddities with the current code are fixed, a new issue arises: now, the deferred completers are added individually to the reader. When #promptWithCompleter() attempts to remove the instance-bound completer from the reader, nothing happens as this is an AggregateCompleter wrapping the lone deferred completer, which was itself added to the reader. Thus, any deferment to be done must still be done with the AggregateCompleter, obviating the deferred completers concept entirely.

              mbenson_jira Matt Benson (Inactive)
              mbenson_jira Matt Benson (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Created:
                Updated:
                Resolved: