Impossible (?) code path resulting in IllegalStateException on jdk14+

[Dawid Weiss]

Hello David, Martin, Doug,

> AFAICS the FJP is critical here as it is the unexpected exception from managedBlock() that causes the problem.

Correct.

Also, something has changed in the behavior of streams in JDK14 that I
haven't tracked down that causes
the "size" of the FJ pool to grow until it explodes. The example I
provided is distilled from a (much) more
complex scenario where the underlying queue was concurrently (and
repeatedly) drained and cleared,
yet the "size" (as reported by toString() - not the active thread set)
of the fj pool still gradually increased
and eventually resulted in the (masked) RejectedExecutionException. I
looked at the code in FJ pool briefly
but didn't dive deep - it's super interesting to read but also heavily
optimized and complex...

I admit we might have made a mistake of switching to parallel streams
as potentially blocking producers...
But this doesn't change the fact that the exception you can get from
ArrayBlockingQueue is quite unexpected
so perhaps it's worth looking into.

Dawid

On Mon, Dec 14, 2020 at 3:57 AM David Holmes <david.holmes at oracle.com> wrote:
>
> Hi Martin,
>
> On 14/12/2020 12:48 pm, Martin Buchholz wrote:
> >  1. JDK-8258187 <https://bugs.openjdk.java.net/browse/JDK-8258187>
> >
> >
> >   IllegalMonitorStateException in ArrayBlockingQueue
> >
> >
> >
> > It's surprising that you can have a repro which is essentially just
> > simple producer-consumer.  Can we remove the forkjoinpool from the repro
> > (just threads?)
>
> AFAICS the FJP is critical here as it is the unexpected exception from
> managedBlock() that causes the problem.
>
> Cheers,
> David
>
> > On Sun, Dec 13, 2020 at 6:28 PM David Holmes <david.holmes at oracle.com
> > <mailto:david.holmes at oracle.com>> wrote:
> >
> >     Hi Dawid,
> >
> >     This appears to be a bug in AbstractQueuedSynchronizer and FJP
> >     interaction, so cc'ing core-libs as this is not a hotspot issue. Also
> >     cc'ing Martin and Doug as this is a j.u.c bug.
> >
> >     Cheers,
> >     David
> >
> >     On 12/12/2020 12:56 am, Dawid Weiss wrote:
> >      > So, I know what this is. Sort of.
> >      >
> >      > This isn't a compiler error (sigh of relief). It's a kind of
> >     interplay
> >      > between parallel streams (which uses the common ForkJoinPool) and the
> >      > queue's blocking operations.
> >      >
> >      > In our code streams use an op closure which sends items to an
> >      > ArrayBlockingQueue. This queue is being drained by a separate thread.
> >      > Everything works up until a certain moment when this happens on
> >      > queue.put() -- I've modified JDK 16 source code and recompiled it to
> >      > see what's happening:
> >      >
> >      > Suppressed: java.util.concurrent.RejectedExecutionException: Thread
> >      > limit exceeded replacing blocked worker
> >      > at
> >     java.base/java.util.concurrent.ForkJoinPool.tryCompensate(ForkJoinPool.java:1579)
> >      > at
> >     java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3124)
> >      > at
> >     java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(AbstractQueuedSynchronizer.java:1614)
> >      > at
> >     java.base/java.util.concurrent.ArrayBlockingQueue.put(ArrayBlockingQueue.java:371)
> >      >
> >      > This exception causes the try-finally block in ArrayBlockingQueue to
> >      > hit the finally block unexpectedly (without the attempt to re-acquire
> >      > the lock!), eventually leading to the original odd exception I
> >      > reported:
> >      >
> >      > Caused by: java.lang.IllegalMonitorStateException
> >      > at
> >     java.base/java.util.concurrent.locks.ReentrantLock$Sync.tryRelease(ReentrantLock.java:175)
> >      > at
> >     java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1006)
> >      > at
> >     java.base/java.util.concurrent.locks.ReentrantLock.unlock(ReentrantLock.java:494)
> >      > at
> >     java.base/java.util.concurrent.ArrayBlockingQueue.put(ArrayBlockingQueue.java:378)
> >      >
> >      > This is then propagated up to the caller of queue.put()
> >      >
> >      > A full simplified repro (without streams) is here:
> >      > https://gist.github.com/dweiss/4787b7aa503ef5702e94d73178c3c842
> >     <https://gist.github.com/dweiss/4787b7aa503ef5702e94d73178c3c842>
> >      >
> >      > When you run it under JDK14+, it'll result in:
> >      >
> >      > java.lang.IllegalMonitorStateException
> >      >          at
> >     java.base/java.util.concurrent.locks.ReentrantLock$Sync.tryRelease(ReentrantLock.java:175)
> >      > ...
> >      >
> >      > This code works on JDK11, by the way. What I find a bit
> >      > counterintuitive is that the original test (code) doesn't make any
> >      > explicit use of ForkJoinPool - it just collects items from a parallel
> >      > stream and involves throwing random exceptions from ops on that
> >      > stream... This was a bit unexpected.
> >      >
> >      > Dawid
> >      >
> >      >
> >      > On Thu, Dec 10, 2020 at 5:25 PM Dawid Weiss
> >     <dawid.weiss at gmail.com <mailto:dawid.weiss at gmail.com>> wrote:
> >      >>
> >      >> Hello,
> >      >>
> >      >> I'm scratching my head again over a bug we encountered in randomized
> >      >> tests. The code is quite complex so before I start to try to
> >     isolate I
> >      >> thought I'd ask if it rings a bell for anybody.
> >      >>
> >      >> The bug is reproducible for certain seeds but happens only after
> >     some
> >      >> VM warmup - the same test is executed a few dozen times, then the
> >      >> problem starts showing up. This only happens on jdk 14 and jdk 15
> >      >> (didn't test on jdk 16 branch). Looks like something related to OSR/
> >      >> compilation races.
> >      >>
> >      >> In the end, we get this exception:
> >      >>
> >      >> java.lang.IllegalMonitorStateException
> >      >> at
> >     java.base/java.util.concurrent.locks.ReentrantLock$Sync.tryRelease(ReentrantLock.java:175)
> >      >> at
> >     java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1006)
> >      >> at
> >     java.base/java.util.concurrent.locks.ReentrantLock.unlock(ReentrantLock.java:494)
> >      >> at
> >     java.base/java.util.concurrent.ArrayBlockingQueue.put(ArrayBlockingQueue.java:373)
> >      >> [stack omitted]
> >      >>
> >      >> This doesn't seem possible from Java code alone -- it's this snippet
> >      >> in ArrayBlockingQueue:
> >      >>
> >      >> lock.lockInterruptibly();
> >      >> try {
> >      >>      while (count == items.length)
> >      >>          notFull.await();
> >      >>      enqueue(e);
> >      >> } finally {
> >      >>     lock.unlock();     // <<< bam...
> >      >> }
> >      >>
> >      >> If the code entered the lock-protected block it should never throw
> >      >> IllegalMonitorStateException, right?
> >      >>
> >      >> I'll start digging in spare moments but hints at how to isolate
> >     this/
> >      >> verify what this can be (other than bisecting git repo) would be
> >     very
> >      >> welcome!
> >      >>
> >      >> Dawid
> >