+ * Basically, it is a segmented read-write lock ({@link StampedLock}), with an additional 'lock for hierarchy update' + * locking mode ({@link #lockForHierarchyUpdate(int)}). + * This is not a generally applicable lock by any means: it is very much tailored for the specific needs of {@link FSRecordsImpl}. + *
+ * The lock is NOT reentrant (because {@link StampedLock} is not reentrant), and an attempt to lock already locked fileId + * down the stack leads to deadlock -- so one needs to be quite careful to use this lock. + * 'Lock for hierarchy update' mode is also not reentrant. + * + * @see StampedLock + */ class FileRecordLock { private static final int SEGMENTS_COUNT = 16; private static final int SEGMENTS_MASK = 0b1111; - private final StampedLock[] segmentedLock = new StampedLock[SEGMENTS_COUNT]; + private final Segment[] segments = new Segment[SEGMENTS_COUNT]; { for (int i = 0; i < SEGMENTS_COUNT; i++) { - segmentedLock[i] = new StampedLock(); + segments[i] = new Segment(); } } public long lockForWrite(int fileId) { - StampedLock lock = lockFor(fileId); + StampedLock lock = segmentFor(fileId); return lock.writeLock(); } - public void unlockForWrite(int fileId, long stamp) { - StampedLock lock = lockFor(fileId); + public void unlockForWrite(int fileId, + long stamp) { + StampedLock lock = segmentFor(fileId); lock.unlockWrite(stamp); } public long lockForRead(int fileId) { - StampedLock lock = lockFor(fileId); + StampedLock lock = segmentFor(fileId); return lock.readLock(); } public void unlockForRead(int fileId, long stamp) { - StampedLock lock = lockFor(fileId); + StampedLock lock = segmentFor(fileId); lock.unlockRead(stamp); } - public StampedLock lockFor(int fileId){ - return segmentedLock[fileId & SEGMENTS_MASK]; + public StampedLock lockFor(int fileId) { + return segmentFor(fileId); + } + + /** + * Locks fileId for "hierarchy update": any attempt to lock same fileId for hierarchy update will be blocked until fileId is + * released with {@link #unlockForHierarchyUpdate(int)} call. + *
+ * 'Hierarchy update' locking mode is independent of regular read/write locking: i.e. fileId locked for hierarchy update is not + * locked for read or write, and could be locked for read/write independently. + *
+ * Hierarchy update lock is NOT reentrant: an attempt to lock the same fileId for hierarchy update down the stack in the same + * thread lead to deadlock. + */ + public void lockForHierarchyUpdate(int fileId) { + segmentFor(fileId).lockHierarchy(fileId); + } + + public void unlockForHierarchyUpdate(int fileId) { + segmentFor(fileId).unlockHierarchy(fileId); + } + + + private Segment segmentFor(int fileId) { + return segments[fileId & SEGMENTS_MASK]; + } + + private static class Segment extends StampedLock { + + /** Set of fileId for which hierarchy updates are now ongoing, so those id are 'locked' for hierarchy updates now */ + private final IntSet hierarchyUpdatesInProcess = new IntOpenHashSet(); + + public void lockHierarchy(int id) { + for (int turn = 0; ; turn++) { + long lockStamp = writeLock(); + try { + if (!hierarchyUpdatesInProcess.contains(id)) { + hierarchyUpdatesInProcess.add(id); + return; + } + + //use active spinning, since stamped lock doesn't support Condition to await()/signal() on: + if (turn < 64) { + Thread.onSpinWait(); + } + else { + LockSupport.parkNanos(1000); + } + } + finally { + unlockWrite(lockStamp); + } + } + } + + public void unlockHierarchy(int id) { + long lockStamp = writeLock(); + try { + boolean actuallyRemoved = hierarchyUpdatesInProcess.remove(id); + if (!actuallyRemoved) { + throw new IllegalStateException("Trying to unlock(" + id + ") which is not currently locked " + hierarchyUpdatesInProcess); + } + } + finally { + unlockWrite(lockStamp); + } + } } } diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PerFileIdLock.java b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PerFileIdLock.java deleted file mode 100644 index e5730f449337..000000000000 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PerFileIdLock.java +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. -package com.intellij.openapi.vfs.newvfs.persistent; - -import it.unimi.dsi.fastutil.ints.IntOpenHashSet; -import it.unimi.dsi.fastutil.ints.IntSet; - -import java.util.concurrent.locks.Condition; -import java.util.concurrent.locks.ReentrantLock; - -/** - * Lock for protecting access to file records (identified by integer fileId) - * Attempt to lock ID that is already locked made the current thread waiting until that ID is unlocked. - *
- * This lock is used in VFS to protect hierarchy updates -- i.e. add/remove/update children - *
- * Lock is NOT re-entrant: an attempt to lock same fileId in the thread that already locked the fileId -- leads to deadlock. - */ -final class PerFileIdLock { - - //TODO RC: Currently we use PerFileIdLock for updating file hierarchy -- which is relatively long process, since it - // involves requests to underlying FS, IO, and children modification. Which is why 'ReentrantLock with fileId - // list per segment' approach was used -- it allows to keep particular fileId locked for some time, without - // locking other fileIds, even those falling into the same segment. The downside is that it is relatively expensive, - // and not reentrant -- both limiting its applicability as general file-record locking method. - - //Ideally, each fileId should have its own lock, but this is too expensive, so we use segmented lock - - private final SegmentLock[] segments; - - PerFileIdLock() { - this(16); - } - - PerFileIdLock(int segmentsCount) { - segments = new SegmentLock[segmentsCount]; - for (int i = 0; i < segments.length; i++) { - segments[i] = new SegmentLock(); - } - } - - public void lock(int id) { - int index = toIndex(id); - segments[index].lock(id); - } - - public void unlock(int id) { - int index = toIndex(id); - segments[index].unlock(id); - } - - private int toIndex(int id) { - return id % segments.length; - } - - private static class SegmentLock { - private final ReentrantLock lock = new ReentrantLock(); - private final Condition unlockCondition = lock.newCondition(); - private final IntSet lockedIds = new IntOpenHashSet(); - - public void lock(int id) { - lock.lock(); - try { - while (lockedIds.contains(id)) { - unlockCondition.awaitUninterruptibly(); - } - lockedIds.add(id); - } - finally { - lock.unlock(); - } - } - - public void unlock(int id) { - lock.lock(); - try { - boolean actuallyRemoved = lockedIds.remove(id); - if (actuallyRemoved) { - //This wakes up all threads waiting -- i.e. lockedIds.size() -- we assume it is usually just a few of them. - // But there could be pathological scenarios there a lot of threads waiting: and each thread will need to - // re-acquire lock, check lockedIds.contains() -- and all threads but one return to waiting after the check. - unlockCondition.signalAll(); - } - else { - throw new IllegalStateException("Trying to unlock(" + id + ") which is not currently locked " + lockedIds); - } - } - finally { - lock.unlock(); - } - } - } -} diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/PersistentFsTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/PersistentFsTest.java index 3253b0934a05..586d06ccab52 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/PersistentFsTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/persistent/PersistentFsTest.java @@ -3,7 +3,6 @@ package com.intellij.openapi.vfs.newvfs.persistent; import com.intellij.CacheSwitcher; import com.intellij.ide.plugins.DynamicPluginsTestUtil; -import com.intellij.idea.IJIgnore; import com.intellij.openapi.Disposable; import com.intellij.openapi.application.Application; import com.intellij.openapi.application.ApplicationManager; @@ -35,7 +34,6 @@ import com.intellij.testFramework.*; import com.intellij.testFramework.fixtures.BareTestFixtureTestCase; import com.intellij.testFramework.rules.TempDirectory; import com.intellij.testFramework.utils.vfs.CheckVFSHealthRule; -import com.intellij.testFramework.utils.vfs.SkipVFSHealthCheck; import com.intellij.util.ArrayUtil; import com.intellij.util.PathUtil; import com.intellij.util.containers.ContainerUtil; @@ -667,7 +665,6 @@ public class PersistentFsTest extends BareTestFixtureTestCase { } @Test - @SkipVFSHealthCheck public void testConcurrentListAllDoesntCauseDuplicateFileIds() throws Exception { PersistentFSImpl pfs = (PersistentFSImpl)PersistentFS.getInstance(); Application application = ApplicationManager.getApplication(); @@ -980,7 +977,7 @@ public class PersistentFsTest extends BareTestFixtureTestCase { events.clear(); } - @IJIgnore(issue = "IJPL-149673") + //@IJIgnore(issue = "IJPL-149673") @Test public void testChildMove() throws IOException { final File firstDirIoFile = tempDirectory.newDirectory("dir1");