On Tue, 25 Feb 2025 01:24:54 GMT, Stuart Marks <sma...@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
