IllegalMonitorStateException in ReentrantLock$Sync.tryRelease on executor shutdown

[szpak]

Working on JMH in #35 I have encountered an exception thrown on an executor service shutdown if ReactorThreadFactory is used with rejectBlocking: false (and daemon: false in that case):

OpenJDK 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended
result
java.lang.IllegalMonitorStateException
	at java.base/java.util.concurrent.locks.ReentrantLock$Sync.tryRelease(ReentrantLock.java:149)
	at java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1302)
	at java.base/java.util.concurrent.locks.ReentrantLock.unlock(ReentrantLock.java:439)
	at java.base/java.util.concurrent.LinkedBlockingQueue.take(LinkedBlockingQueue.java:440)
	at java.base/java.util.concurrent.ThreadPoolExecutor.getTask(ThreadPoolExecutor.java:1054)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1114)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)

It occurs only with BlockHound installed. Sample code to produce that result:

public class ReproduceIllegalMonitorStateException {

    public static void main(String[] args) throws ExecutionException, InterruptedException {
        ThreadFactory blockingUnfriendlyThreadFactory = new ReactorThreadFactory("blocking-unfriendly", new AtomicLong(), false, true,
                (t, e) -> e.printStackTrace());
        ExecutorService executor = Executors.newSingleThreadExecutor(blockingUnfriendlyThreadFactory);;
        try {
            BlockHound.install();
            System.out.println(executor.submit(() -> "result").get());
        } finally {
            executor.shutdown(); //fails with non daemon thread with "java.lang.IllegalMonitorStateException" on "ReentrantLock$Sync.tryRelease()"
        }
    }
}

[bsideup]

Hi @szpak,

Thanks for reporting! 👍 I confirm that there is a blocking call detected:

java.lang.Exception: Blocking call: jdk.internal.misc.Unsafe#park
	at reactor.core.scheduler.Blah.lambda$main$1(Blah.java:20)
	at reactor.blockhound.BlockHound$Builder.lambda$install$8(BlockHound.java:259)
	at reactor.blockhound.BlockHoundRuntime.checkBlocking(BlockHoundRuntime.java:43)
	at java.base/jdk.internal.misc.Unsafe.park(Unsafe.java)
	at java.base/java.util.concurrent.locks.LockSupport.park(LockSupport.java:194)
	at java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(AbstractQueuedSynchronizer.java:2081)
	at java.base/java.util.concurrent.LinkedBlockingQueue.take(LinkedBlockingQueue.java:433)
	at java.base/java.util.concurrent.ThreadPoolExecutor.getTask(ThreadPoolExecutor.java:1054)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1114)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:835)

Although since this is ThreadPoolExecutor's internals, I'm keen to whitelist it. WDYT?

FYI sometimes exceptions get swallowed (like here), but you can use this trick:

BlockHound.install(b -> {
    b.blockingMethodCallback(m -> {
        new Exception("Blocking call: " + m).printStackTrace();
    });
});

[szpak]

Yes, in the meantime I also got the root cause of that issue (with blockingMethodCallback()) and I just wanted to paste the same stacktrace :).

Regarding whitelisting, as my case is legitimate - Callable submitted from a normal thread - I don't see any other clear way to do not break that code.

[szpak]

The whitelisting has disadvantage that I cannot simply test non-blocking thread, no "markers" in the stacktrace scenario :).
In general, in production code, all of the blocking calls in non-blocking threads will be ignored if run though a executor.

[bsideup]

You can test it without the executor. In fact, I would not use the executor here (nor any Reactor's code)

[szpak]

On the other hand, whitelisting allowBlockingCallsInside("java.util.concurrent.ThreadPoolExecutor", "getTask"); probably not all the calls will be ignored.

Update. Response to myself. GH didn't refresh the page.

[szpak]

You can test it without the executor. In fact, I would not use the executor here (nor any Reactor's code)

Currently I have my own ThreadFactory and I do not reuse Reactor's code. However, it's handy to use single thread ThreadPoolExecutor to reuse one thread in thousand of calls (within one JMH iteration). What do you propose to use instead?