diff --git a/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java b/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java index e9da6d9e2232..e469b42eca97 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java @@ -1221,7 +1221,7 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App ActivityTracker.getInstance().inc(); fireBeforeWriteActionStart(clazz); - if (!myLock.isWriteLocked() && !myLock.tryWriteLock()) { + if (!myLock.isWriteLocked()) { Future reportSlowWrite = ourDumpThreadsOnLongWriteActionWaiting <= 0 ? null : JobScheduler.getScheduler() .scheduleWithFixedDelay(() -> PerformanceWatcher.getInstance().dumpThreads("waiting", true), diff --git a/platform/platform-impl/src/com/intellij/openapi/application/impl/ReadMostlyRWLock.java b/platform/platform-impl/src/com/intellij/openapi/application/impl/ReadMostlyRWLock.java index f637cc16f23d..1757cd68eca7 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/impl/ReadMostlyRWLock.java +++ b/platform/platform-impl/src/com/intellij/openapi/application/impl/ReadMostlyRWLock.java @@ -71,6 +71,16 @@ class ReadMostlyRWLock { Reader(@NotNull Thread readerThread) { thread = readerThread; } + + @Override + public String toString() { + return "Reader{" + + "thread=" + thread + + ", readRequested=" + readRequested + + ", blocked=" + blocked + + ", impatientReads=" + impatientReads + + '}'; + } } private final ThreadLocal R = ThreadLocal.withInitial(() -> { @@ -97,7 +107,7 @@ class ReadMostlyRWLock { for (int iter = 0; ; iter++) { if (tryReadLock(status, true)) { - return; + break; } ProgressManager.checkCanceled(); @@ -181,6 +191,7 @@ class ReadMostlyRWLock { } private static final int SPIN_TO_WAIT_FOR_LOCK = 100; + void writeLock() { checkWriteThreadAccess(); assert !writeRequested; @@ -292,21 +303,6 @@ class ReadMostlyRWLock { } } - boolean tryWriteLock() { - checkWriteThreadAccess(); - assert !writeRequested; - assert !writeAcquired; - - writeRequested = true; - if (areAllReadersIdle()) { - writeAcquired = true; - return true; - } - - writeRequested = false; - return false; - } - private boolean areAllReadersIdle() { for (Reader reader : readers) { if (reader.readRequested) { @@ -322,4 +318,16 @@ class ReadMostlyRWLock { } static class SuspensionId {} + + @Override + public String toString() { + return "ReadMostlyRWLock{" + + "writeThread=" + writeThread + + ", writeRequested=" + writeRequested + + ", writeAcquired=" + writeAcquired + + ", readers=" + readers + + ", privilegedReaders=" + privilegedReaders + + ", currentSuspension=" + currentSuspension + + '}'; + } } diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/ApplicationImplTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/ApplicationImplTest.java index 055287818ad6..18874f650ab6 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/ApplicationImplTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/ApplicationImplTest.java @@ -16,6 +16,7 @@ package com.intellij.openapi.application.impl; import com.intellij.concurrency.JobSchedulerImpl; +import com.intellij.diagnostic.ThreadDumper; import com.intellij.openapi.Disposable; import com.intellij.openapi.application.*; import com.intellij.openapi.application.ex.ApplicationEx; @@ -43,9 +44,9 @@ import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; -@SuppressWarnings("StatementWithEmptyBody") @RunFirst public class ApplicationImplTest extends LightPlatformTestCase { @Override @@ -144,24 +145,20 @@ public class ApplicationImplTest extends LightPlatformTestCase { for (int i = 0; i < numOfThreads; i++) { Thread thread = new Thread(() -> { assertFalse(application.isReadAccessAllowed()); - //System.out.println("start "+Thread.currentThread()); for (int i1 = 0; i1 < readIterations; i1++) { application.runReadAction(() -> { }); } - //System.out.println("end "+Thread.currentThread()); }, "read thread " + i); thread.start(); threads.add(thread); } if (writeIterations > 0) { - //System.out.println("write start"); for (int i = 0; i < writeIterations; i++) { ApplicationManager.getApplication().runWriteAction(() -> { }); } - //System.out.println("write end"); } ConcurrencyUtil.joinAll(threads); threads.clear(); @@ -173,10 +170,9 @@ public class ApplicationImplTest extends LightPlatformTestCase { } private long timeOut; - private boolean ok() throws Throwable { + private void checkTimeout() throws Throwable { if (exception != null) throw exception; - if (System.currentTimeMillis() > timeOut) throw new RuntimeException("timeout"); - return true; + if (System.currentTimeMillis() > timeOut) throw new TimeoutException("timeout"); } public void testAppLockReadWritePreference() throws Throwable { @@ -206,7 +202,7 @@ public class ApplicationImplTest extends LightPlatformTestCase { try { LOG.debug("read lock1 acquired"); read1Acquired.set(true); - while (holdRead1.get() && ok()); + while (holdRead1.get()) checkTimeout(); } finally { read1Released.set(true); @@ -221,15 +217,15 @@ public class ApplicationImplTest extends LightPlatformTestCase { }, "read lock1"); readAction1.start(); - while (!read1Acquired.get() && ok()); + while (!read1Acquired.get()) checkTimeout(); AtomicBoolean aboutToAcquireWrite = new AtomicBoolean(); // readActions2 should try to acquire read action when write action is pending Thread readActions2 = new Thread(() -> { try { assertFalse(application.isDispatchThread()); - while (!aboutToAcquireWrite.get() && ok()); + while (!aboutToAcquireWrite.get()) checkTimeout(); // make sure EDT called writelock - while (!application.myLock.writeRequested && ok()); + while (!application.myLock.writeRequested) checkTimeout(); assertTrue(application.isWriteActionPending()); //assertFalse(application.tryRunReadAction(EmptyRunnable.getInstance())); AccessToken stamp = application.acquireReadActionLock(); @@ -254,13 +250,14 @@ public class ApplicationImplTest extends LightPlatformTestCase { Thread checkThread = new Thread(()->{ try { assertFalse(application.isDispatchThread()); - while (!aboutToAcquireWrite.get() && ok()); - while (!read1Acquired.get() && ok()); + while (!aboutToAcquireWrite.get()) checkTimeout(); + while (!read1Acquired.get()) checkTimeout(); // make sure EDT called writelock - while (!application.myLock.writeRequested && ok()); + while (!application.myLock.writeRequested) checkTimeout(); long timeout = System.currentTimeMillis() + 2_000; - while (System.currentTimeMillis() < timeout && ok()) { + while (System.currentTimeMillis() < timeout) { + checkTimeout(); assertTrue(aboutToAcquireWrite.get()); assertTrue(read1Acquired.get()); assertFalse(read1Released.get()); @@ -276,10 +273,11 @@ public class ApplicationImplTest extends LightPlatformTestCase { } holdRead1.set(false); - while (!writeAcquired.get() && ok()); + while (!writeAcquired.get()) checkTimeout(); timeout = System.currentTimeMillis() + 2_000; - while (System.currentTimeMillis() < timeout && ok()) { + while (System.currentTimeMillis() < timeout) { + checkTimeout(); assertTrue(aboutToAcquireWrite.get()); assertTrue(read1Acquired.get()); assertTrue(read1Released.get()); @@ -296,10 +294,11 @@ public class ApplicationImplTest extends LightPlatformTestCase { holdWrite.set(false); - while (!read2Released.get() && ok()); + while (!read2Released.get()) checkTimeout(); timeout = System.currentTimeMillis() + 2_000; - while (System.currentTimeMillis() < timeout && ok()) { + while (System.currentTimeMillis() < timeout) { + checkTimeout(); assertTrue(aboutToAcquireWrite.get()); assertTrue(read1Acquired.get()); assertTrue(read1Released.get()); @@ -326,7 +325,8 @@ public class ApplicationImplTest extends LightPlatformTestCase { LOG.debug("write lock acquired"); writeAcquired.set(true); - while (holdWrite.get() && ok()) { + while (holdWrite.get()) { + checkTimeout(); assertTrue(application.isWriteActionInProgress()); assertTrue(application.isWriteAccessAllowed()); assertFalse(application.isWriteActionPending()); @@ -362,7 +362,7 @@ public class ApplicationImplTest extends LightPlatformTestCase { ApplicationManager.getApplication().runReadAction((ThrowableComputable)() -> { LOG.append("inside read action\n"); readStarted = true; - while (!tryingToStartWriteAction && ok()) ; + while (!tryingToStartWriteAction) checkTimeout(); TimeoutUtil.sleep(100); readThreads = ContainerUtil.map(anotherReadActionStarted, readActionStarted -> new Thread(() -> { @@ -387,7 +387,7 @@ public class ApplicationImplTest extends LightPlatformTestCase { readThreads.forEach(Thread::start); for (AtomicBoolean threadStarted : anotherThreadStarted) { - while (!threadStarted.get() && ok()) ; + while (!threadStarted.get()) checkTimeout(); } // now the other threads try to get read lock. we should not let them for (int i = 0; i < 10; i++) { @@ -408,7 +408,7 @@ public class ApplicationImplTest extends LightPlatformTestCase { main.start(); - while (!readStarted && ok()); + while (!readStarted) checkTimeout(); tryingToStartWriteAction = true; LOG.append("\nwrite about to start"); ApplicationManager.getApplication().runWriteAction(() -> { @@ -440,7 +440,6 @@ public class ApplicationImplTest extends LightPlatformTestCase { assertFalse(ApplicationManager.getApplication().isReadAccessAllowed()); assertFalse(ApplicationManager.getApplication().isDispatchThread()); for (int i=0; i<100;i++) { - //noinspection SSBasedInspection SwingUtilities.invokeLater(() -> ApplicationManager.getApplication().runWriteAction(() -> TimeoutUtil.sleep(20))); ApplicationManager.getApplication().runReadAction(() -> TimeoutUtil.sleep(20)); } @@ -505,7 +504,6 @@ public class ApplicationImplTest extends LightPlatformTestCase { } public void testRunProcessWithProgressSynchronouslyInReadActionWithPendingWriteAction() throws Throwable { - //noinspection SSBasedInspection SwingUtilities.invokeLater(() -> ApplicationManager.getApplication().runWriteAction(EmptyRunnable.getInstance())); AtomicBoolean ran = new AtomicBoolean(); boolean result = ((ApplicationEx)ApplicationManager.getApplication()) @@ -714,7 +712,7 @@ public class ApplicationImplTest extends LightPlatformTestCase { app.runReadAction(() -> { readAcquired.set(true); try { - while (!stopRead.get() && ok()); + while (!stopRead.get()) checkTimeout(); } catch (Throwable e) { exception = e; @@ -724,14 +722,20 @@ public class ApplicationImplTest extends LightPlatformTestCase { } }) ); - while (!readAcquired.get()); + while (!readAcquired.get()) checkTimeout(); + + AtomicBoolean writeCompleted = new AtomicBoolean(); Future readAction2 = app.executeOnPooledThread(() -> { try { // wait for write action attempt to start - i.e. app.myLock.writeLock() started to execute - while (!app.myLock.writeRequested && ok()); + while (!app.myLock.writeRequested) checkTimeout(); app.executeByImpatientReader(() -> { try { + assertFalse(app.isReadAccessAllowed()); app.runReadAction(EmptyRunnable.getInstance()); + assertFalse(writeCompleted.get()); + if (exception != null) throw new RuntimeException(exception); + System.out.println(ThreadDumper.dumpThreadsToString()); fail("Must have been failed"); } catch (ApplicationUtil.CannotRunReadActionException ignored) { @@ -751,6 +755,7 @@ public class ApplicationImplTest extends LightPlatformTestCase { }); app.runWriteAction(EmptyRunnable.getInstance()); + writeCompleted.set(true); readAction2.get(); readAction1.get(); @@ -765,7 +770,7 @@ public class ApplicationImplTest extends LightPlatformTestCase { app.runReadAction(() -> { readAcquired.set(true); try { - while (!stopRead.get() && ok()) ; + while (!stopRead.get()) checkTimeout(); } catch (Throwable e) { exception = e; @@ -775,14 +780,20 @@ public class ApplicationImplTest extends LightPlatformTestCase { } }) ); - while (!readAcquired.get()); + while (!readAcquired.get()) checkTimeout(); AtomicBoolean executingImpatientReader = new AtomicBoolean(); - AtomicBoolean readAction2CalledReadAction = new AtomicBoolean(); Future readAction2 = app.executeOnPooledThread(() -> { // wait for write action attempt to start - while (!app.myLock.writeRequested); + while (!app.myLock.writeRequested) { + try { + checkTimeout(); + } + catch (Throwable e) { + throw new RuntimeException(e); + } + } ProgressManager.getInstance().executeNonCancelableSection(()-> app.executeByImpatientReader(() -> { executingImpatientReader.set(true); app.runReadAction(EmptyRunnable.getInstance()); @@ -792,7 +803,7 @@ public class ApplicationImplTest extends LightPlatformTestCase { Future readAction1Canceler = app.executeOnPooledThread(() -> { try { - while (!executingImpatientReader.get() && ok()); + while (!executingImpatientReader.get()) checkTimeout(); // make sure readAction2 does call runReadAction() TimeoutUtil.sleep(300); stopRead.set(true);