Hi Leonid,

On 27/03/2020 7:39 am, Leonid Mesnik wrote:
Replying with correct summary.

Leonid

On 3/23/20 8:55 PM, Leonid Mesnik wrote:
Hi

Could you please review following fix which update ThreadsRunner to use AtomicInteger/spinOnWait instead of Wicket to synchronize starting of stress test threads.

Failing tests allocated all memory by earlier started threads before Lock.unlock is called in the latest threads. So thread might get an OOME exception while trying to release lock and/or get into inconsistent state.

You have a bug in Wicket:

+        try {
+            lock.lock();
...
+        } finally {
+            lock.unlock();

The lock() has to go outside the try block. That is why you were getting IllegalMonitorStateExceptions when the lock() threw OOME.

But the OOME itself is still a problem as it means you can't use any proper synchronizer. I don't like seeing the spin-loops but in this code you may have no choice if memory may already be exhausted.

David
-----



The bug was introduced by https://bugs.openjdk.java.net/browse/JDK-8241123 <https://bugs.openjdk.java.net/browse/JDK-8241123> The Atomic works fine for stress test finishing sync. I just didn't expect that tests might OOME while releasing start lock. Verified that tests now don't fail with -Xcomp -server -XX:-TieredCompilation -XX:-UseCompressedOops.

webrev: http://cr.openjdk.java.net/~lmesnik/8241456/webrev.00/ <http://cr.openjdk.java.net/~lmesnik/8241456/webrev.00/> bug: https://bugs.openjdk.java.net/browse/JDK-8241456 <https://bugs.openjdk.java.net/browse/JDK-8241456>

Leonid

Reply via email to