From 0c4cb96af7bee3eae3230522b1966d03a16890f7 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Thu, 31 May 2018 17:21:25 +0300 Subject: [PATCH] cleanup: fewer error-prone start/stop methods --- .../com/intellij/psi/stubs/StubIndexImpl.java | 26 ++---- .../util/indexing/FileBasedIndexImpl.java | 90 ++++++++----------- .../util/indexing/IndexAccessValidator.java | 15 +++- .../com/intellij/util/ConcurrencyUtil.java | 23 +++++ 4 files changed, 77 insertions(+), 77 deletions(-) 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 2425ca616d84..14f185d954fe 100644 --- a/platform/lang-impl/src/com/intellij/psi/stubs/StubIndexImpl.java +++ b/platform/lang-impl/src/com/intellij/psi/stubs/StubIndexImpl.java @@ -328,30 +328,20 @@ public class StubIndexImpl extends StubIndex implements PersistentStateComponent @NotNull StubIdListContainerAction action) { final FileBasedIndexImpl fileBasedIndex = (FileBasedIndexImpl)FileBasedIndex.getInstance(); ID stubUpdatingIndexId = StubUpdatingIndex.INDEX_ID; - myAccessValidator.checkAccessingIndexDuringOtherIndexProcessing(stubUpdatingIndexId); final MyIndex index = (MyIndex)getAsyncState().myIndices.get(indexKey); // wait for initialization to finish fileBasedIndex.ensureUpToDate(stubUpdatingIndexId, project, scope); UpdatableIndex stubUpdatingIndex = fileBasedIndex.getIndex(stubUpdatingIndexId); try { - myAccessValidator.checkAccessingIndexDuringOtherIndexProcessing(stubUpdatingIndexId); - try { - // disable up-to-date check to avoid locks on attempt to acquire index write lock while holding at the same time the readLock for this index - FileBasedIndexImpl.disableUpToDateCheckForCurrentThread(); - stubUpdatingIndex.getReadLock().lock(); - - myAccessValidator.startedProcessingActivityForIndex(stubUpdatingIndexId); - - return index.getData(key).forEach(action); + // disable up-to-date check to avoid locks on attempt to acquire index write lock while holding at the same time the readLock for this index + return FileBasedIndexImpl.disableUpToDateCheckIn(()-> + myAccessValidator.validate(stubUpdatingIndexId, ()->index.getData(key).forEach(action))); } finally { - myAccessValidator.stoppedProcessingActivityForIndex(stubUpdatingIndexId); stubUpdatingIndex.getReadLock().unlock(); - FileBasedIndexImpl.enableUpToDateCheckForCurrentThread(); - wipeProblematicFileIdsForParticularKeyAndStubIndex(indexKey, key, stubUpdatingIndex); } } @@ -427,11 +417,9 @@ public class StubIndexImpl extends StubIndex implements PersistentStateComponent final MyIndex index = (MyIndex)getAsyncState().myIndices.get(indexKey); // wait for initialization to finish FileBasedIndex.getInstance().ensureUpToDate(StubUpdatingIndex.INDEX_ID, scope.getProject(), scope); - myAccessValidator.checkAccessingIndexDuringOtherIndexProcessing(StubUpdatingIndex.INDEX_ID); try { - myAccessValidator.startedProcessingActivityForIndex(StubUpdatingIndex.INDEX_ID); - FileBasedIndexImpl.disableUpToDateCheckForCurrentThread(); - return index.processAllKeys(processor, scope, idFilter); + return FileBasedIndexImpl.disableUpToDateCheckIn(()-> + myAccessValidator.validate(StubUpdatingIndex.INDEX_ID, () -> index.processAllKeys(processor, scope, idFilter))); } catch (StorageException e) { forceRebuild(e); @@ -443,10 +431,6 @@ public class StubIndexImpl extends StubIndex implements PersistentStateComponent } throw e; } - finally { - FileBasedIndexImpl.enableUpToDateCheckForCurrentThread(); - myAccessValidator.stoppedProcessingActivityForIndex(StubUpdatingIndex.INDEX_ID); - } return true; } diff --git a/platform/lang-impl/src/com/intellij/util/indexing/FileBasedIndexImpl.java b/platform/lang-impl/src/com/intellij/util/indexing/FileBasedIndexImpl.java index 9468cb81851a..d44dd77fe6fb 100644 --- a/platform/lang-impl/src/com/intellij/util/indexing/FileBasedIndexImpl.java +++ b/platform/lang-impl/src/com/intellij/util/indexing/FileBasedIndexImpl.java @@ -660,12 +660,21 @@ public class FileBasedIndexImpl extends FileBasedIndex implements BaseComponent, private static final ThreadLocal myUpToDateCheckState = new ThreadLocal<>(); - public static void disableUpToDateCheckForCurrentThread() { + public static T disableUpToDateCheckIn(@NotNull ThrowableComputable runnable) throws E { + disableUpToDateCheckForCurrentThread(); + try { + return runnable.compute(); + } + finally { + enableUpToDateCheckForCurrentThread(); + } + } + private static void disableUpToDateCheckForCurrentThread() { final Integer currentValue = myUpToDateCheckState.get(); myUpToDateCheckState.set(currentValue == null ? 1 : currentValue.intValue() + 1); } - public static void enableUpToDateCheckForCurrentThread() { + private static void enableUpToDateCheckForCurrentThread() { final Integer currentValue = myUpToDateCheckState.get(); if (currentValue != null) { final int newValue = currentValue.intValue() - 1; @@ -882,17 +891,7 @@ public class FileBasedIndexImpl extends FileBasedIndex implements BaseComponent, //assert project != null : "GlobalSearchScope#getProject() should be not-null for all index queries"; ensureUpToDate(indexId, project, filter, restrictToFile); - myAccessValidator.checkAccessingIndexDuringOtherIndexProcessing(indexId); - - try { - index.getReadLock().lock(); - myAccessValidator.startedProcessingActivityForIndex(indexId); - return computable.convert(index); - } - finally { - myAccessValidator.stoppedProcessingActivityForIndex(indexId); - index.getReadLock().unlock(); - } + return ConcurrencyUtil.withLock(index.getReadLock(), ()->myAccessValidator.validate(indexId, ()->computable.convert(index))); } catch (StorageException e) { scheduleRebuild(indexId, e); @@ -997,13 +996,7 @@ public class FileBasedIndexImpl extends FileBasedIndex implements BaseComponent, final MapReduceIndex index = (MapReduceIndex)state.getIndex(indexId); assert index != null; final MemoryIndexStorage memStorage = (MemoryIndexStorage)index.getStorage(); - index.getReadLock().lock(); - try { - memStorage.clearCaches(); - } - finally { - index.getReadLock().unlock(); - } + ConcurrencyUtil.withLock(index.getReadLock(), () -> memStorage.clearCaches()); } } @@ -1441,13 +1434,7 @@ public class FileBasedIndexImpl extends FileBasedIndex implements BaseComponent, final MapReduceIndex index = (MapReduceIndex)state.getIndex(indexId); assert index != null; final MemoryIndexStorage memStorage = (MemoryIndexStorage)index.getStorage(); - index.getWriteLock().lock(); - try { - memStorage.clearMemoryMap(); - } - finally { - index.getWriteLock().unlock(); - } + ConcurrencyUtil.withLock(index.getWriteLock(), () -> memStorage.clearMemoryMap()); memStorage.fireMemoryStorageCleared(); } } @@ -1727,8 +1714,7 @@ public class FileBasedIndexImpl extends FileBasedIndex implements BaseComponent, } private void scheduleUpdate(@NotNull final ID indexId, @NotNull Computable update, VirtualFile file, final int inputId, final boolean hasContent) { if (runUpdate(false, update)) { - myReadLock.lock(); - try { + ConcurrencyUtil.withLock(myReadLock, ()->{ UpdatableIndex index = getIndex(indexId); if (hasContent) { index.setIndexedStateForFile(inputId, file); @@ -1736,10 +1722,7 @@ public class FileBasedIndexImpl extends FileBasedIndex implements BaseComponent, else { index.resetIndexedStateForFile(inputId); } - } - finally { - myReadLock.unlock(); - } + }); } } @@ -2031,26 +2014,27 @@ public class FileBasedIndexImpl extends FileBasedIndex implements BaseComponent, myWorkersFinishedSync.register(); int phase = myWorkersFinishedSync.getPhase(); try { - myVfsEventsMerger.processChanges(info -> { - myWriteLock.lock(); - try { - ProgressManager.getInstance().executeNonCancelableSection(() -> { - int fileId = info.getFileId(); - VirtualFile file = info.getFile(); - if (info.isTransientStateChanged()) FileBasedIndexImpl.this.doTransientStateChangeForFile(fileId, file); - if (info.isBeforeContentChanged()) FileBasedIndexImpl.this.doInvalidateIndicesForFile(fileId, file, true); - if (info.isContentChanged()) scheduleFileForIndexing(fileId, file, true); - if (info.isFileRemoved()) FileBasedIndexImpl.this.doInvalidateIndicesForFile(fileId, file, false); - if (info.isFileAdded()) scheduleFileForIndexing(fileId, file, false); - }); - } - finally { - IndexingStamp.flushCache(info.getFileId()); - myWriteLock.unlock(); - } - return true; - }); - } finally { + myVfsEventsMerger.processChanges(info -> + ConcurrencyUtil.withLock(myWriteLock, () -> { + try { + ProgressManager.getInstance().executeNonCancelableSection(() -> { + int fileId = info.getFileId(); + VirtualFile file = info.getFile(); + if (info.isTransientStateChanged()) FileBasedIndexImpl.this.doTransientStateChangeForFile(fileId, file); + if (info.isBeforeContentChanged()) FileBasedIndexImpl.this.doInvalidateIndicesForFile(fileId, file, true); + if (info.isContentChanged()) scheduleFileForIndexing(fileId, file, true); + if (info.isFileRemoved()) FileBasedIndexImpl.this.doInvalidateIndicesForFile(fileId, file, false); + if (info.isFileAdded()) scheduleFileForIndexing(fileId, file, false); + }); + } + finally { + IndexingStamp.flushCache(info.getFileId()); + } + return true; + }) + ); + } + finally { myWorkersFinishedSync.arriveAndDeregister(); } diff --git a/platform/lang-impl/src/com/intellij/util/indexing/IndexAccessValidator.java b/platform/lang-impl/src/com/intellij/util/indexing/IndexAccessValidator.java index 8e60e074f747..ba00050c9105 100644 --- a/platform/lang-impl/src/com/intellij/util/indexing/IndexAccessValidator.java +++ b/platform/lang-impl/src/com/intellij/util/indexing/IndexAccessValidator.java @@ -17,6 +17,7 @@ package com.intellij.util.indexing; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.util.ThrowableComputable; import org.jetbrains.annotations.NotNull; import java.text.MessageFormat; @@ -24,7 +25,7 @@ import java.text.MessageFormat; public class IndexAccessValidator { private final ThreadLocal> ourAlreadyProcessingIndices = new ThreadLocal<>(); - public void checkAccessingIndexDuringOtherIndexProcessing(@NotNull ID indexKey) { + private void checkAccessingIndexDuringOtherIndexProcessing(@NotNull ID indexKey) { final ID alreadyProcessingIndex = ourAlreadyProcessingIndices.get(); if (alreadyProcessingIndex != null && alreadyProcessingIndex != indexKey) { final String message = MessageFormat.format("Accessing ''{0}'' during processing ''{1}''. Nested different indices processing may cause deadlock", @@ -35,6 +36,14 @@ public class IndexAccessValidator { } } - public void startedProcessingActivityForIndex(ID indexId) { ourAlreadyProcessingIndices.set(indexId); } - public void stoppedProcessingActivityForIndex(ID indexId) { ourAlreadyProcessingIndices.set(null); } + public T validate(@NotNull ID indexKey, @NotNull ThrowableComputable runnable) throws E { + checkAccessingIndexDuringOtherIndexProcessing(indexKey); + ourAlreadyProcessingIndices.set(indexKey); + try { + return runnable.compute(); + } + finally { + ourAlreadyProcessingIndices.set(null); + } + } } diff --git a/platform/util/src/com/intellij/util/ConcurrencyUtil.java b/platform/util/src/com/intellij/util/ConcurrencyUtil.java index ba3b4800b313..38a7298a7b69 100644 --- a/platform/util/src/com/intellij/util/ConcurrencyUtil.java +++ b/platform/util/src/com/intellij/util/ConcurrencyUtil.java @@ -17,6 +17,7 @@ package com.intellij.util; import com.intellij.ReviseWhenPortedToJDK; import com.intellij.diagnostic.ThreadDumper; +import com.intellij.openapi.util.ThrowableComputable; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.TestOnly; @@ -25,6 +26,7 @@ import java.util.*; import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; /** @@ -232,4 +234,25 @@ public class ConcurrencyUtil { } }; } + + public static T withLock(@NotNull Lock lock, @NotNull ThrowableComputable runnable) throws E { + lock.lock(); + try { + return runnable.compute(); + } + finally { + lock.unlock(); + } + } + + public static void withLock(@NotNull Lock lock, @NotNull ThrowableRunnable runnable) throws E { + lock.lock(); + try { + runnable.run(); + } + finally { + lock.unlock(); + } + } + }