RFR: 8267952: async logging supports to dynamically change tags and decorators [v3]

[Xin Liu]

On Mon, 14 Jun 2021 04:51:28 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> Support dynamic reconfiguration for async logging. 2 unittests are provided.
>> The regression test discovers a race condition in LogTagSet::log() even with
>> synchronous logging. It's not MT-safe if context switch happens between the
>> creation of LogDecorations  and LogOutputList::Iterator. fixed.
>
> 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 three additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8267952
>  - Add comments in LogTagSet::log().
>  - 8267952: async logging supports to dynamically change tags and decorators

hi, Kim, 

Thanks for the head-up. 

I did prepare a patch to support Atomic::store. I think I followed what you guys did. 
https://cr.openjdk.java.net/~xliu/8267952/atomic_store_LogDecorators.diff

Because this patch uses some C++ tricks(well, advanced techniques), I have to convince myself that it's necessary. so far, I found that it always generates the same store instructions in x86_64.

@dholmes-ora 
The biggest concern is the MT-safety. Users can potentially change unified logging configurations anytime in the presence of dcmd. That's why I added 2 concurrent gtests to demonstrate it's MT-safe no matter it's synchronous logging or async logging.  We must be sure that SEGV fault never happen. 

As far as I known, there are 2 reasons why we can see that crash sight. 

1.  `LogConfiguration::configure_output` may be still lack of a memory barrier somewhere. 
Some logs directed to output[idx] can still enqueue after `AsyncLogWriter::flush()`. delete_output(idx) is the hazard. 

If so, synchronous logging should observe the same problem. The fact is we only observed the crash in AsyncLog Thread. 
Further, I ran `LogConfigurationTest.reconfigure_tags_MT` a couple of hours on x86_64 and aarch64 individually.  I still can't capture the crash. 

2. LogFileOutput::rotate() may nullify _stream. 
yes, I missed bugfix JDK-8268165, but this patch uses "filecount=0" to prevent log from rotating in  `LogConfigurationTest.reconfigure_tags_MT`.  In my understanding, rotate() should not be invoked by this test.  

I have merged the master branch, which includes 8268165. Could you try it again and see if it will still crash at the same place?

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

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