RFR: 8267926: AsyncLogGtest.java fails on assert with: decorator was not part of the decorator set specified at creation. [v3]

[Liu Xin]

hi, David,

Thanks.  Let's see the results of tier5~7 with the basic fix.  If it can
pass all tests, it means that it's wrong to hoist AsyncLogWriter::flush().

Actually, I didn't change a lot in this PR. I hoisted the
AsyncLogWriter::flush() because in my understanding one flush() can remove
two hazards in async mode.
https://openjdk.github.io/cr/?repo=jdk&pr=4257&range=00-01#udiff-1-src/hotspot/share/logging/logConfiguration.cpp

Yesterday, when you ran the tier3~4 test, did all failures occur on aarch64?
If so, my only explanation is that this code may not be safe on aarch64.
That doesn't make sense because OrderAccess::storeload() is a full memory
barrier on aarch64.

void LogOutputList::wait_until_no_readers() const {
  OrderAccess::storeload();
  while (_active_readers != 0) {
    // Busy wait
  }
}

So far, I still can't reproduce the crash you sent me yesterday on aarch64.
I tried different configurations and even used cset to simulate different
numbers of cores.







On Tue, Jun 1, 2021 at 8:03 PM David Holmes <david.holmes at oracle.com> wrote:

> On 2/06/2021 2:01 am, Daniel D.Daugherty wrote:
> > On Mon, 31 May 2021 17:09:48 GMT, Xin Liu <xliu at openjdk.org> wrote:
> >
> >>> The root cause of the intermittent failure is that _decorators in
> LogDecorations
> >>> may be inconsistent with LogOutput::_decorators. It could happen when
> gtest disables a
> >>> Log output via set_log_config(TestLogFileName, "all=off").
> >>>
> >>> Since we copy the entire logDecorations, it's reasonable to copy
> _decorator as well.
> >>> LogDecorator is a bitmask of uint. It's even smaller than a reference
> on LP64 platforms.
> >>>
> >>> This patch not only fixes intermittent crash in JDK-8267926, but also
> fixes JDK-8267952.
> >>> Two hazards of async logging in `LogConfiguration::configure_output`
> are eliminated.
> >>> This patch also adds a storestore barrier to make sure it's safe on
> weak consistency machines.
> >>
> >> Xin Liu has updated the pull request incrementally with one additional
> commit since the last revision:
> >>
> >>    Increase reader's counter before creating LogDecoration.
> >
> > It looks like this failure is taking a bit of work to resolve. I've gone
> ahead and
> > ProblemListed the test. At this point we have 20 sightings in Tier[3-7].
> Since
> > this failure is intermittent, I would want to see Tier[5-7] testing done
> before
> > the fix is integrated.
>
> I've submitted the basic fix for tiers 5-7 testing.
>
> Xin: note you will need to merge with master and remove the test from
> the Problem-list again as part of this PR.
>
> Thanks,
> David
> -----
>
> > -------------
> >
> > PR: https://git.openjdk.java.net/jdk/pull/4257
> >
>