diff --git a/java/java-tests/testSrc/com/intellij/util/indexing/IndexTest.java b/java/java-tests/testSrc/com/intellij/util/indexing/IndexTest.java index 822ca7340d9b..8efe8d7ab67e 100644 --- a/java/java-tests/testSrc/com/intellij/util/indexing/IndexTest.java +++ b/java/java-tests/testSrc/com/intellij/util/indexing/IndexTest.java @@ -750,26 +750,31 @@ public class IndexTest extends JavaCodeInsightFixtureTestCase { return true; }); - assertTrue(foundClass.get(0)); assertTrue(foundMethod.get(0));// allow access stub index processing other index final List foundClassProcessAll = new ArrayList<>(List.of(false)); final List foundClassStub = new ArrayList<>(List.of(false)); - StubIndex.getInstance().processAllKeys(JavaStubIndexKeys.CLASS_SHORT_NAMES, getProject(), aClass -> { - if (!className.equals(aClass)) return true; - foundClassProcessAll.set(0, true); - StubIndex.getInstance() - .processElements(JavaStubIndexKeys.CLASS_SHORT_NAMES, aClass, getProject(), scope, PsiClass.class, clazz -> { - foundClassStub.set(0, true); - return true; - }); - return true; - }); + try { + StubIndex.getInstance().processAllKeys(JavaStubIndexKeys.CLASS_SHORT_NAMES, getProject(), aClass -> { + if (!className.equals(aClass)) return true; + foundClassProcessAll.set(0, true); + StubIndex.getInstance() + .processElements(JavaStubIndexKeys.CLASS_SHORT_NAMES, aClass, getProject(), scope, PsiClass.class, clazz -> { + foundClassStub.set(0, true); + return true; + }); + return true; + }); + fail("IllegalStateException should be thrown in tests"); + } + catch (IllegalStateException e) { + assertEquals("Nesting processElements call under other stub index operation can lead to a deadlock.", e.getMessage()); + } assertTrue(foundClassProcessAll.get(0)); - assertTrue(foundClassStub.get(0)); + assertFalse(foundClassStub.get(0)); final List foundId = new ArrayList<>(List.of(false)); final List foundStub = new ArrayList<>(List.of(false)); @@ -970,21 +975,26 @@ public class IndexTest extends JavaCodeInsightFixtureTestCase { final List foundClassProcessAll = new ArrayList<>(List.of(false)); final List foundClassStub = new ArrayList<>(List.of(false)); - StubIndex.getInstance().processAllKeys(JavaStubIndexKeys.CLASS_SHORT_NAMES, getProject(), aClass -> { - if (!className.equals(aClass)) return true; - foundClassProcessAll.set(0, true); - // adding file will add file to index's dirty set, but it should not be processed within current read action - myFixture.addFileToProject("Bar.java", "class Bar { }"); - StubIndex.getInstance() - .processElements(JavaStubIndexKeys.CLASS_SHORT_NAMES, aClass, getProject(), scope, PsiClass.class, clazz -> { - foundClassStub.set(0, true); - return true; - }); - return true; - }); + try { + StubIndex.getInstance().processAllKeys(JavaStubIndexKeys.CLASS_SHORT_NAMES, getProject(), aClass -> { + if (!className.equals(aClass)) return true; + foundClassProcessAll.set(0, true); + // adding file will add file to index's dirty set, but it should not be processed within current read action + myFixture.addFileToProject("Bar.java", "class Bar { }"); + StubIndex.getInstance() + .processElements(JavaStubIndexKeys.CLASS_SHORT_NAMES, aClass, getProject(), scope, PsiClass.class, clazz -> { + foundClassStub.set(0, true); + return true; + }); + return true; + }); + fail("IllegalStateException should be thrown in tests"); + } catch (IllegalStateException e) { + assertEquals("Nesting processElements call under other stub index operation can lead to a deadlock.", e.getMessage()); + } assertTrue(foundClassProcessAll.get(0)); - assertTrue(foundClassStub.get(0));// allow access stub index processing other index + assertFalse(foundClassStub.get(0));// do not allow access stub index processing other index } public void test_document_increases_beyond_too_large_limit() { @@ -1420,7 +1430,8 @@ public class IndexTest extends JavaCodeInsightFixtureTestCase { fileBasedIndex.ensureUpToDate(trigramId, getProject(), GlobalSearchScope.everythingScope(getProject())); assertEmpty(fileBasedIndex.getIndex(trigramId).getIndexedFileData(fileId).values()); - IndexingRequestToken indexingRequest = getProject().getService(ProjectIndexingDependenciesService.class).getLatestIndexingRequestToken(); + IndexingRequestToken indexingRequest = + getProject().getService(ProjectIndexingDependenciesService.class).getLatestIndexingRequestToken(); assertFalse(IndexingFlag.isFileIndexed(file, indexingRequest.getFileIndexingStamp(file))); } @@ -1462,7 +1473,8 @@ public class IndexTest extends JavaCodeInsightFixtureTestCase { public void test_stub_index_updated_after_language_level_change() { VirtualFile file = myFixture.addFileToProject("src1/A.java", "class A {}").getVirtualFile(); - FilePropertyKey javaLanguageLevelKey = FilePropertyPusher.EP_NAME.findExtension(JavaLanguageLevelPusher.class).getFilePropertyKey(); + FilePropertyKey javaLanguageLevelKey = + FilePropertyPusher.EP_NAME.findExtension(JavaLanguageLevelPusher.class).getFilePropertyKey(); LanguageLevel languageLevel = javaLanguageLevelKey.getPersistentValue(file.getParent()); assertNotNull(languageLevel); @@ -1531,7 +1543,8 @@ public class IndexTest extends JavaCodeInsightFixtureTestCase { VirtualFile scratchTxt = ScratchRootType.getInstance().createScratchFile(getProject(), "Foo.txt", PlainTextLanguage.INSTANCE, "xxx"); assertTrue(ScratchesSearchScope.getScratchesScope(getProject()).contains(scratchTxt)); - GlobalSearchScope scratchJava = GlobalSearchScope.getScopeRestrictedByFileTypes(ScratchesSearchScope.getScratchesScope(getProject()), JavaFileType.INSTANCE); + GlobalSearchScope scratchJava = + GlobalSearchScope.getScopeRestrictedByFileTypes(ScratchesSearchScope.getScratchesScope(getProject()), JavaFileType.INSTANCE); assertFalse(scratchJava.contains(scratchTxt)); VirtualFileEnumeration scratchJavaEnum = VirtualFileEnumeration.extract(scratchJava); assertNotNull(scratchJavaEnum); @@ -1550,6 +1563,7 @@ public class IndexTest extends JavaCodeInsightFixtureTestCase { var processor = new Processor() { final AtomicInteger processed = new AtomicInteger(0); + @Override public boolean process(String i) { processed.incrementAndGet(); diff --git a/platform/indexing-impl/src/com/intellij/psi/stubs/StubIndexEx.java b/platform/indexing-impl/src/com/intellij/psi/stubs/StubIndexEx.java index 7c5dd5b8d6ed..99bd86d15ec3 100644 --- a/platform/indexing-impl/src/com/intellij/psi/stubs/StubIndexEx.java +++ b/platform/indexing-impl/src/com/intellij/psi/stubs/StubIndexEx.java @@ -136,6 +136,15 @@ public abstract class StubIndexEx extends StubIndex { @Nullable IdFilter idFilter, @NotNull Class requiredClass, @NotNull Processor processor) { + if (!canUpdateAllIndexes() || !getStubUpdatingIndex().canUpdate()) { + var exception = new IllegalStateException("Nesting processElements call under other stub index operation can lead to a deadlock."); + if (ApplicationManager.getApplication().isUnitTestMode()) { + throw exception; + } + else { + getLogger().error(exception); + } + } var trace = lookupStubEntriesStarted(indexKey) .withProject(project); @@ -242,6 +251,8 @@ public abstract class StubIndexEx extends StubIndex { @ApiStatus.Internal protected abstract UpdatableIndex getIndex(@NotNull StubIndexKey indexKey); + protected abstract boolean canUpdateAllIndexes(); + // Self repair for IDEA-181227, caused by (yet) unknown file event processing problem in indices // FileBasedIndex.requestReindex doesn't handle the situation properly because update requires old data that was lost private void tryFixIndexesForProblemFiles(@NotNull StubIndexKey indexKey, @NotNull Key key, @NotNull Project project) { diff --git a/platform/indexing-impl/src/com/intellij/util/indexing/UpdatableIndex.java b/platform/indexing-impl/src/com/intellij/util/indexing/UpdatableIndex.java index 9bff841fd004..5aedbb70fe13 100644 --- a/platform/indexing-impl/src/com/intellij/util/indexing/UpdatableIndex.java +++ b/platform/indexing-impl/src/com/intellij/util/indexing/UpdatableIndex.java @@ -62,4 +62,9 @@ public interface UpdatableIndex extends In void cleanupForNextTest(); boolean isDirty(); + + @Internal + default boolean canUpdate() { + return true; + } } diff --git a/platform/indexing-impl/src/com/intellij/util/indexing/impl/storage/TransientChangesIndexStorage.java b/platform/indexing-impl/src/com/intellij/util/indexing/impl/storage/TransientChangesIndexStorage.java index cbd3f7af704b..1eee386d7bd4 100644 --- a/platform/indexing-impl/src/com/intellij/util/indexing/impl/storage/TransientChangesIndexStorage.java +++ b/platform/indexing-impl/src/com/intellij/util/indexing/impl/storage/TransientChangesIndexStorage.java @@ -307,4 +307,9 @@ public final class TransientChangesIndexStorage implements VfsAwareI // and persistent storage return inMemoryStorage.size() + underlyingStorage.keysCountApproximately(); } + + @Override + public boolean isReadLockHeldByCurrentThread() { + return underlyingStorage.isReadLockHeldByCurrentThread(); + } } diff --git a/platform/indexing-impl/src/com/intellij/util/indexing/storage/fake/FakeStorageLayout.kt b/platform/indexing-impl/src/com/intellij/util/indexing/storage/fake/FakeStorageLayout.kt index 9d4376a6cd0a..6ee74daafde8 100644 --- a/platform/indexing-impl/src/com/intellij/util/indexing/storage/fake/FakeStorageLayout.kt +++ b/platform/indexing-impl/src/com/intellij/util/indexing/storage/fake/FakeStorageLayout.kt @@ -66,7 +66,7 @@ internal class FakeIndexStorage : VfsAwareIndexStorage { override fun read(key: K?, processor: ValueContainerProcessor): Boolean { return processor.process(ValueContainer.emptyContainer()) } - + override fun flush() = Unit override fun close() = Unit @@ -86,4 +86,6 @@ internal class FakeIndexStorage : VfsAwareIndexStorage { override fun isClosed(): Boolean = false override fun keysCountApproximately(): Int = 0 + + override fun isReadLockHeldByCurrentThread(): Boolean = false } diff --git a/platform/indexing-impl/src/com/intellij/util/indexing/storage/sharding/ShardedIndexStorage.java b/platform/indexing-impl/src/com/intellij/util/indexing/storage/sharding/ShardedIndexStorage.java index 539c20978c79..30f66b687a9f 100644 --- a/platform/indexing-impl/src/com/intellij/util/indexing/storage/sharding/ShardedIndexStorage.java +++ b/platform/indexing-impl/src/com/intellij/util/indexing/storage/sharding/ShardedIndexStorage.java @@ -4,6 +4,7 @@ package com.intellij.util.indexing.storage.sharding; import com.intellij.openapi.util.ThrowableNotNullFunction; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.util.Processor; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.indexing.FileBasedIndexExtension; import com.intellij.util.indexing.IdFilter; import com.intellij.util.indexing.StorageException; @@ -170,4 +171,9 @@ class ShardedIndexStorage implements VfsAwareIndexStorage { public boolean isClosed() { return closed; } + + @Override + public boolean isReadLockHeldByCurrentThread() { + return ContainerUtil.or(shards, IndexStorage::isReadLockHeldByCurrentThread); + } } diff --git a/platform/lang-impl/src/com/intellij/psi/stubs/StubIndexImpl.java b/platform/lang-impl/src/com/intellij/psi/stubs/StubIndexImpl.java index 52b5674e410d..690c8ed8acba 100644 --- a/platform/lang-impl/src/com/intellij/psi/stubs/StubIndexImpl.java +++ b/platform/lang-impl/src/com/intellij/psi/stubs/StubIndexImpl.java @@ -223,6 +223,11 @@ public final class StubIndexImpl extends StubIndexEx { return (UpdatableIndex)getAsyncState().myIndices.get(indexKey); } + @Override + protected boolean canUpdateAllIndexes() { + return ContainerUtil.all(getAsyncState().myIndices.values(), it -> it.canUpdate()); + } + @Override public void forceRebuild(@NotNull Throwable e) { FileBasedIndex.getInstance().requestRebuild(StubUpdatingIndex.INDEX_ID, e); diff --git a/platform/util/src/com/intellij/util/indexing/impl/IndexStorage.java b/platform/util/src/com/intellij/util/indexing/impl/IndexStorage.java index ba8af66d8f40..7715ba248c68 100644 --- a/platform/util/src/com/intellij/util/indexing/impl/IndexStorage.java +++ b/platform/util/src/com/intellij/util/indexing/impl/IndexStorage.java @@ -113,4 +113,7 @@ public interface IndexStorage extends Flushable, Closeable { default void invalidateCachedMappings() { //nothing } + + @Internal + boolean isReadLockHeldByCurrentThread(); } diff --git a/platform/util/src/com/intellij/util/indexing/impl/IndexStorageLockingBase.java b/platform/util/src/com/intellij/util/indexing/impl/IndexStorageLockingBase.java index dcec34ddbddd..1ec101c5ac72 100644 --- a/platform/util/src/com/intellij/util/indexing/impl/IndexStorageLockingBase.java +++ b/platform/util/src/com/intellij/util/indexing/impl/IndexStorageLockingBase.java @@ -34,9 +34,21 @@ public abstract class IndexStorageLockingBase { */ public static final boolean MAKE_INDEX_LOOKUP_CANCELLABLE = getBooleanProperty("intellij.index.cancellable-lookup", true); + private final ThreadLocal readLockCount = new ThreadLocal<>(); private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); //cache the handles to avoid allocations on fast path: - private transient final LockStamp readLockUnlockHandle = lock.readLock()::unlock; + private transient final LockStamp readLockUnlockHandle = () -> { + Integer count = readLockCount.get(); + if (count != null) { + if (count <= 1) { + readLockCount.remove(); + } + else { + readLockCount.set(count - 1); + } + } + lock.readLock().unlock(); + }; private transient final LockStamp writeLockUnlockHandle = lock.writeLock()::unlock; protected IndexStorageLockingBase() { @@ -50,7 +62,7 @@ public abstract class IndexStorageLockingBase { else { readLock.lock(); } - + readLockCount.set(readLockCount.get() == null ? 1 : readLockCount.get() + 1); return readLockUnlockHandle; } @@ -77,7 +89,16 @@ public abstract class IndexStorageLockingBase { } } + @ApiStatus.Internal + public boolean isReadLockHeldByCurrentThread() { + Integer count = readLockCount.get(); + return count != null && count > 0; + } + protected @NotNull LockStamp lockForWrite() { + if (isReadLockHeldByCurrentThread()) { + throw new IllegalStateException("Cannot acquire write lock while read lock is held"); + } ReentrantReadWriteLock.WriteLock writeLock = lock.writeLock(); writeLock.lock(); return writeLockUnlockHandle; diff --git a/platform/util/src/com/intellij/util/indexing/impl/MapReduceIndex.java b/platform/util/src/com/intellij/util/indexing/impl/MapReduceIndex.java index f36c3457e343..2e898c447c5b 100644 --- a/platform/util/src/com/intellij/util/indexing/impl/MapReduceIndex.java +++ b/platform/util/src/com/intellij/util/indexing/impl/MapReduceIndex.java @@ -367,6 +367,11 @@ public abstract class MapReduceIndex implements InvertedIndex protected abstract void requestRebuild(@NotNull Throwable e); + @Internal + public boolean canUpdate() { + return !myStorage.isReadLockHeldByCurrentThread(); + } + private final UpdatedEntryProcessor changedEntriesProcessor = new UpdatedEntryProcessor() { @Override public void process(@NotNull UpdateKind kind, Key key, Value value, int inputId) throws StorageException {