[stubs] IJPL-2989 Prevent nested write in IndexStorageLockingBase and log an error in case of nested processElements call.

GitOrigin-RevId: eea1d9fd89fb63ba50c096588f29ceecd5551165
This commit is contained in:
Piotr Tomiak
2025-09-30 17:17:31 +00:00
committed by intellij-monorepo-bot
parent 608f8eeca4
commit 666c18a47b
10 changed files with 108 additions and 31 deletions
@@ -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<Boolean> foundClassProcessAll = new ArrayList<>(List.of(false));
final List<Boolean> 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<Boolean> foundId = new ArrayList<>(List.of(false));
final List<Boolean> foundStub = new ArrayList<>(List.of(false));
@@ -970,21 +975,26 @@ public class IndexTest extends JavaCodeInsightFixtureTestCase {
final List<Boolean> foundClassProcessAll = new ArrayList<>(List.of(false));
final List<Boolean> 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<LanguageLevel> javaLanguageLevelKey = FilePropertyPusher.EP_NAME.findExtension(JavaLanguageLevelPusher.class).getFilePropertyKey();
FilePropertyKey<LanguageLevel> 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<String>() {
final AtomicInteger processed = new AtomicInteger(0);
@Override
public boolean process(String i) {
processed.incrementAndGet();
@@ -136,6 +136,15 @@ public abstract class StubIndexEx extends StubIndex {
@Nullable IdFilter idFilter,
@NotNull Class<Psi> requiredClass,
@NotNull Processor<? super Psi> 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 <Key> UpdatableIndex<Key, Void, FileContent, ?> getIndex(@NotNull StubIndexKey<Key, ?> 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 <Key> void tryFixIndexesForProblemFiles(@NotNull StubIndexKey<Key, ?> indexKey, @NotNull Key key, @NotNull Project project) {
@@ -62,4 +62,9 @@ public interface UpdatableIndex<Key, Value, Input, FileIndexMetaData> extends In
void cleanupForNextTest();
boolean isDirty();
@Internal
default boolean canUpdate() {
return true;
}
}
@@ -307,4 +307,9 @@ public final class TransientChangesIndexStorage<Key, Value> implements VfsAwareI
// and persistent storage
return inMemoryStorage.size() + underlyingStorage.keysCountApproximately();
}
@Override
public boolean isReadLockHeldByCurrentThread() {
return underlyingStorage.isReadLockHeldByCurrentThread();
}
}
@@ -66,7 +66,7 @@ internal class FakeIndexStorage<K, V> : VfsAwareIndexStorage<K, V> {
override fun <E : Exception?> read(key: K?, processor: ValueContainerProcessor<V?, E?>): Boolean {
return processor.process(ValueContainer.emptyContainer())
}
override fun flush() = Unit
override fun close() = Unit
@@ -86,4 +86,6 @@ internal class FakeIndexStorage<K, V> : VfsAwareIndexStorage<K, V> {
override fun isClosed(): Boolean = false
override fun keysCountApproximately(): Int = 0
override fun isReadLockHeldByCurrentThread(): Boolean = false
}
@@ -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<K, V> implements VfsAwareIndexStorage<K, V> {
public boolean isClosed() {
return closed;
}
@Override
public boolean isReadLockHeldByCurrentThread() {
return ContainerUtil.or(shards, IndexStorage::isReadLockHeldByCurrentThread);
}
}
@@ -223,6 +223,11 @@ public final class StubIndexImpl extends StubIndexEx {
return (UpdatableIndex<Key, Void, FileContent, ?>)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);
@@ -113,4 +113,7 @@ public interface IndexStorage<Key, Value> extends Flushable, Closeable {
default void invalidateCachedMappings() {
//nothing
}
@Internal
boolean isReadLockHeldByCurrentThread();
}
@@ -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<Integer> 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;
@@ -367,6 +367,11 @@ public abstract class MapReduceIndex<Key, Value, Input> implements InvertedIndex
protected abstract void requestRebuild(@NotNull Throwable e);
@Internal
public boolean canUpdate() {
return !myStorage.isReadLockHeldByCurrentThread();
}
private final UpdatedEntryProcessor<Key, Value> changedEntriesProcessor = new UpdatedEntryProcessor<Key, Value>() {
@Override
public void process(@NotNull UpdateKind kind, Key key, Value value, int inputId) throws StorageException {