Uploaded image for project: 'logback'
  1. logback
  2. LOGBACK-791

Design issue: Should an OnConsoleStatusListener be addable to the StatusManager if there already is one?

    XMLWordPrintable

Details

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Major Major
    • None
    • None
    • None
    • None

    Description

      • We configure logback through logback.xml
      • Initial, pre-configuration state of logback has an "OnConsoleStatusListener" set up.

      In effect, if I understand what I see in the debugger, once the "configuration" tag is encountered, the following call stack happens:

      Interpreter.startElement() calls
      Interpreter.callBeginAction() calls
      ConfigurationAction.begin() calls
      OnConsoleStatusListener.addNewInstanceToContext(context);

      which creates a new OnConsoleStatusListener, starts it, then adds it to the StatusManager reachable via the context.

      At the moment, the first messages appear on the Console as the "retrospective" is printed out if the messages in there are not too old.

      Now:

      If logback.xml contains

      <statusListener class="ch.qos.logback.core.status.OnConsoleStatusListener" />

      then a second OnConsoleStatusListener instance will be added to the StatusManager reachable via the context.

      This leads to a confusing duplication of messages. First all the retrospective, which may have grown since the first printout, is printed. All subsequent status messages are then printed, too.

      Would it not be better to check in

      OnConsoleStatusListener.addNewInstanceToContext(Context context)

      whether a OnConsoleStatusListener already exists in the StatusManager (in this case, the ch.qos.logback.core.BasicStatusManager) and do nothing if that is so?

      This is a special handling of OnConsoleStatusListener, of course, barring some more general idea (like a flag on the listener saying "I am responsible for the console and there can be only one").

      There is the nasty problem that the new OnConsoleStatusListener must not be started before we are sure it could be added, because starting makes it print out the retrospective. But start() is not in the interface of StatusListener.

      This thus seems to demand that BasicStatusManager.add() is specially tuned to OnConsoleStatusListener:

      -------------
      public void add(StatusListener listener) {
      if (listener != null) {
      synchronized (statusListenerListLock) {
      boolean addIt = true;
      // Check that there is no OnConsoleStatusListener already
      // If not, start it before adding it
      if (listener instanceof OnConsoleStatusListener) {
      for (StatusListener cur : statusListenerList) {
      if (cur instanceof OnConsoleStatusListener)

      { addIt = false; break; }

      }
      if (addIt)

      { ((OnConsoleStatusListener) listener).start(); }

      }
      if (addIt)

      { statusListenerList.add(listener); }

      }
      }
      }
      --------

      And that OnConsoleStatusListener leave "start()" to the "add()" method. This is not fully nice, because the hidden assumption is that the StatusManager returned by the Context is indeed a BasicStatusManager. The correct solution would be to have start() defined on StatusListener and call the add() method addAndStart(), but that would change the interface:

      --------
      static public void addNewInstanceToContext(Context context)

      { OnConsoleStatusListener onConsoleStatusListener = new OnConsoleStatusListener(); onConsoleStatusListener.setContext(context); // onConsoleStatusListener.start(); context.getStatusManager().add(onConsoleStatusListener); }

      --------

      Attachments

        Activity

          People

            logback-dev Logback dev list
            dtonhofer David Tonhofer
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated: