RFR: 8349206: j.u.l.Handler classes create deadlock risk via synchronized publish() method [v9]

[Jason Mehrens]

On Tue, 25 Feb 2025 01:24:54 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> As for the example you gave there, that is interesting. Keeping an unformatted log record around for any time after the log statement that created it has exited would be quite problematic (it prevents GC of arbitrary things).
>> 
>> If I were doing some kind of summary tail entry, I'd pull, format (user args) and cache anything I needed out of the log record when I had it, but not keep it around. Then when the tail is written I'd just have what I need ready to go.
>> 
>> But yes, right now, with or without this PR, that code looks like it's got a deadlock risk.
>
> Need to update to have single call to getFormatter().

>Keeping an unformatted log record around for any time after the log statement that created it has exited would be quite problematic (it prevents GC of arbitrary things).

I've always leaned on:
1. The words written in LogRecord API that once a record is given to the logging api it owns that record. Thefore, don't attach things to that record.  "Don't leave your belongings in the rental car when returning it."
3. Passing arbitrary objects to the logging API is a security risk because I can create a filter/handler/Formatter that casts the arguments and manipulates them. E.G. Map::clear, Map::put("user","root").
4. Logging api has supplier methods that allow callers to format arguments early on using String.format.
5. LogRecord records a timestamp. Passing string representation at time of logging both snapshots near the timestamp and makes a mutable argument safer for exposure to unknown classes.

That said, I'll probably create PRs for MailHandler and CollectorFormatter to optionally support deep cloning of the LogRecord via serialization before it is stored. Doing so would switch the params to string without tampering with the source record.  MemoryHander could do the same and effectively become a non-caching (pushLevel=ALL) TransformHandler to decorate Handers that over synchronize.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23491#discussion_r1973650209