fix testReadActionInImpatientModeShouldThrowWhenThereIsAPendingWrite

(during tryWrite() writeRequested was reset to false temporarily, which caused discontinuity in tests assuming write lock is in process of acquiring. That reset didn't make sense anyway because all it did was instill false hope to readers waiting in the other threads)
This commit is contained in:
Alexey Kudravtsev
2018-11-20 18:42:42 +03:00
parent 91c7656afe
commit a276775bb8
3 changed files with 70 additions and 51 deletions
@@ -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),
@@ -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<Reader> 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 +
'}';
}
}
@@ -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<Object, Throwable>)() -> {
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);