Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8349208

MemoryHandler logging class can cause incorrect information to be logged.

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: P4 P4
    • tbd
    • 25
    • core-libs
    • None

      The MemoryHandler logging class has several hard-to-fix issues in its design and implementation, and risks both deadlock and logging incorrect information in the face of concurrent use.

      The deadlock issue is discussed in https://bugs.openjdk.org/browse/JDK-8349206 and while the deadlock issue can be mitigated, that mitigation may exacerbate any concurrency issues in this class relating to logging incorrect information.

      Fundamentally, this class attempts to do something which should never be done with LogRecords, which is "formatting the user supplied log arguments outside the scope of the log statement to which they were given".

      By allowing log records to be buffered (as is) in the thread in which they were logged, and later published (and formatted) to a target handler in another thread, there is an inherent risk of incorrect data being logged.

      Consider:

      var map = new HashMap<String, String>();
      map.put("key1", "foo");
      logger.log(INFO, "Map = {}", map);
      map.put("key2", "bar");

      At the point at which "map" is logged, it contains one element, and this is what any user would expect to appear in the log output.

      However, if MemoryHandler is used, the record can be formatted at some arbitrary later time, and be seen (incorrectly) to contain different elements in the output to the log statement.

      Perhaps worse, the MemoryHandler implementation allows buffered log records to continue to exist, precluding garbage collection of their user supplied parameters, for an arbitrary amount of time (definitely beyond any threads which use the object, and possibly "forever" depending on what other logging occurs).

      Options to address this issue include:
      1. Early formatting of log records (not without potential new issues).
      2. Additional user guidance and warnings around using this class.
      3. Formal deprecation, with a view to removing this class completely from the JDK.

      While "early formatting" sounds appealing as an "easy fix", it's worth noting that it effectively "undoes" any performance improvements people would see from using this class (by virtue of moving expensive formatting back into the logging thread) and could arguably remove the user facing benefits of using this class at all (which might be an argument for just deprecating it).

      It also has issues regarding tools which analyze log records in a structured way, since the published log records would now contain strings where they previously contained user parameters, and log messages can now be "out of sync" with respect to the logged parameter types when using non-trivial MessageFormat directives (e.g. "{1,number,$'#',##}").

      Furthermore, and code changes to mitigate this issue should probably also seek to address the deadlock issue raised by https://bugs.openjdk.org/browse/JDK-8349206.

            dabeaumo David Beaumont
            dabeaumo David Beaumont
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: