RFR: 8229517: Support for optional asynchronous/buffered logging [v8]

[Xin Liu]

On Thu, 25 Mar 2021 21:19:05 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> Xin Liu has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 18 additional commits since the last revision:
>> 
>>  - Refactor LogAsyncFlusher::abort()
>>    
>>    This change makes sure LogAsyncFlusher::abort() is lock-less.
>>    Therefore, it's not subject to mutex rank issue. Newly added
>>    gtest(mutex_lock_access_leaf)  may deliberately trigger SIGSEGV
>>    while holding access rank mutex, so abort() must be lockless.
>>  - Merge branch 'master' into JDK-8229517
>>  - Fix a race condition bug on LogAsyncFlusher termination.
>>    
>>    I saw intermitent crashes of java with the following arguments.
>>    -Xlog:'all=trace:file=hotspot-x.log:level,tags:filecount=0,async=true' --version
>>    
>>    The root cause is that there is a race condition between main thread _exit and
>>    LogAsyncFlusher::run. This patch added a synchronization using
>>    Terminator_lock in LogAsyncFlusher::terminate. This guranatees that no
>>    log entry will emit when main thread is exiting.
>>  - Resolve rank conflict between tty_lock and LogAsyncFlusher's _lock.
>>    
>>    LogAsyncFlusher::_lock ranks Mutex::tty on purpose, which is same as tty_lock.
>>    Ideally, they are orthogonal. In reality, it's possible that a thread emits logs
>>    to a log file while (mistakenly) holding tty_lock. ttyUnlocker is placed in enqueue
>>    member functions to resolve conflicts betwen them.
>>    
>>    This patch fixed the jtreg test: runtime/logging/RedefineClasses.java and the
>>    full brunt logging -Xlog:'all:file=hotspot.log::async=true'
>>  - Remove LogAsyncInterval completely
>>    
>>    It was used as a timeout parameter of the monitor. Now the monitor is waked up
>>    when the occupancy of asynclog buffer is more 3/4.
>>  - Support Jcmd pid VM.log disable
>>    
>>    This patch also supports to add a new output dynamically. If
>>    output_option specifies async=true, the new output will use
>>    asynchronous writing.
>>    
>>    Currently jcmd VM.log prohibts users from changing established
>>    output_options in runtime. users can disable them all and then
>>    recreate them with the new output_options.
>>  - Fix build issue with `--disable-precompiled-headers`
>>  - Inject the number of dropped messages since last dumpping.
>>    
>>    Each LogOutput has an independent counter. The out-of-band message
>>    "[number] messages dropped..." will be dumped into its corresponding
>>    LogOutput.
>>  - Revert "fix runtime/logging/RedefineClasses.java crashed with -XX:+AsyncLogging"
>>    
>>    This reverts commit 81b2a0cb2a6cf57b1cd0baacdf8c0419f14819b4.
>>    
>>    This problem is sidetracked by JDK-8265102.
>>  - fix runtime/logging/RedefineClasses.java crashed with -XX:+AsyncLogging
>>    
>>    nmethod::print(outputStream* st) should not obtain tty_lock by assuming
>>    st is defaultStream. It could be logStream as well.
>>    
>>    Currently, AyncLogFlusher::_lock has the same rank of tty_lock.
>>    https://issues.amazon.com/issues/JVM-563
>>  - ... and 8 more: https://git.openjdk.java.net/jdk/compare/1c42ea7a...4cf4a57f
>
> src/hotspot/share/logging/logAsyncFlusher.hpp line 96:
> 
>> 94:     _level(decorations.get_level()), _tagset(decorations.get_logTagSet()) {
>> 95:       // allow to fail here, then _message is NULL
>> 96:       _message = os::strdup(msg, mtLogging);
> 
> If you think `msg` can't be NULL here please add an assertion, otherwise please handle it.

I deliberately design this code snippet tolerate error. 

`os::strdup` uses malloc. The default behavior is returning NULL when it fails. it's fine that _message is NULL.  the payload is "null" and nothing will be emitted eventually.

If I handle error(lacking memory) here, the it may then recursively generates more logs.

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

PR: https://git.openjdk.java.net/jdk/pull/3135