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

Logging stops if invalid xml configuration is loaded by scanner

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.28
    • Fix Version/s: 0.9.30
    • Component/s: None
    • Labels:
      None

      Description

      When logging with scan="true" it is possible to stop all logging of an application silently.

      Steps to reproduce:

      • start an application with valid configuration and scan=true
      • break the logback.xml (leave out a quote or something)
        -> log goes silent

      Expected behavior:
      Logger fails to load new config and keeps running on the old configuration (like when valid xml, but invalid configuration is loaded).

        Activity

        Hide
        iwein Iwein Fuld added a comment -

        The ideal solution is pretty clear to me: load the configuration first, then swap the contexts only if the loading was successful. Even if this doesn't happen entirely thread-safe, it would fix this issue. The relevant question is how to modify logback code to get close enough to this situation to fix this bug. After trying for an hour or so last week I can say it will be trickier than I hoped, but that's normal.

        I came up with some strategies that are not very invasive and open the door to a larger refactoring that attacks the design problem I mentioned.

        1. run the JoranConfigurator against a throwaway context first and only reset after that worked

        2. wrap the context in a delegator that can swap the internal context instead of modifying it

        I haven't tried these strategies yet, but I think they can be made to fix the problem and the second one could be made threadsafe and atomic. Thoughts?

        Show
        iwein Iwein Fuld added a comment - The ideal solution is pretty clear to me: load the configuration first, then swap the contexts only if the loading was successful. Even if this doesn't happen entirely thread-safe, it would fix this issue. The relevant question is how to modify logback code to get close enough to this situation to fix this bug. After trying for an hour or so last week I can say it will be trickier than I hoped, but that's normal. I came up with some strategies that are not very invasive and open the door to a larger refactoring that attacks the design problem I mentioned. 1. run the JoranConfigurator against a throwaway context first and only reset after that worked 2. wrap the context in a delegator that can swap the internal context instead of modifying it I haven't tried these strategies yet, but I think they can be made to fix the problem and the second one could be made threadsafe and atomic. Thoughts?
        Hide
        noreply.ceki@qos.ch Ceki Gulcu added a comment - - edited

        There might be undesirable side effects even when running joran on a temporary context. For example, a RollingFileAppender might open/close an existing file as a result of running a config file.

        Another strategy is to memorize a safe configuration script (as a DOM tree) and fallback to it if the current configuration file turns out to be bogus. However, correctly determining that a configuration file is bogus or alternatively safe can be rather difficult.

        Note that any retained strategy must work correctly in presence of inclusions, i.e. the <include> directive.

        Show
        noreply.ceki@qos.ch Ceki Gulcu added a comment - - edited There might be undesirable side effects even when running joran on a temporary context. For example, a RollingFileAppender might open/close an existing file as a result of running a config file. Another strategy is to memorize a safe configuration script (as a DOM tree) and fallback to it if the current configuration file turns out to be bogus. However, correctly determining that a configuration file is bogus or alternatively safe can be rather difficult. Note that any retained strategy must work correctly in presence of inclusions, i.e. the <include> directive.
        Hide
        rgoers@apache.org Ralph Goers added a comment -

        One way to handle the issue with the files is to abstract out the file handling from the Appender. As appenders are created and destroyed the underlying FileManager remains intact with the stream, connection, etc associated with it.

        Show
        rgoers@apache.org Ralph Goers added a comment - One way to handle the issue with the files is to abstract out the file handling from the Appender. As appenders are created and destroyed the underlying FileManager remains intact with the stream, connection, etc associated with it.
        Hide
        noreply.ceki@qos.ch Ceki Gulcu added a comment -

        This issue is now fixed. See http://github.com/ceki/logback/commit/690ac09e30d01

        Joran will remember well-formed configuration files. In case an XML/SAX parsing error occurs (in the new configuration file), Joran will fall back to previous configuration file. This should work even in case of included configuration files.

        Show
        noreply.ceki@qos.ch Ceki Gulcu added a comment - This issue is now fixed. See http://github.com/ceki/logback/commit/690ac09e30d01 Joran will remember well-formed configuration files. In case an XML/SAX parsing error occurs (in the new configuration file), Joran will fall back to previous configuration file. This should work even in case of included configuration files.
        Show
        tony19 Tony Trinh added a comment - Fixed in https://github.com/qos-ch/logback/commit/690ac09e30d01

          People

          • Assignee:
            noreply.ceki@qos.ch Ceki Gulcu
            Reporter:
            iwein Iwein Fuld
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: