[vcs-log] cancel all progresses on dispose and throw PCE from disposed log storage

It is bad to work with VcsLogStorage after dispose. This can happen if some refreshing/filtering processes
are still in progress when log is disposed. So:
1. All processes are cancelled in VcsLogProgress.
2. VcsLogStorage throws PCE if accessed when disposed.
This commit is contained in:
Julia Beliaeva
2016-09-12 17:20:52 +03:00
parent 210e6844ac
commit cfb7d9af76
5 changed files with 42 additions and 14 deletions
@@ -93,7 +93,7 @@ public class VcsLogData implements Disposable, VcsLogDataProvider {
if (!(e instanceof ProcessCanceledException)) {
LOG.error(e);
}
}, RECENT_COMMITS_COUNT);
}, RECENT_COMMITS_COUNT, this);
myContainingBranchesGetter = new ContainingBranchesGetter(this, this);
}
@@ -26,12 +26,13 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.List;
import java.util.Set;
import java.util.function.Consumer;
public class VcsLogProgress {
public class VcsLogProgress implements Disposable {
@NotNull private final Object myLock = new Object();
@NotNull private final List<ProgressListener> myListeners = ContainerUtil.newArrayList();
private int myRunningTasksCount = 0;
@NotNull private Set<ProgressIndicator> myRunningTasks = ContainerUtil.newHashSet();
@NotNull
public ProgressIndicator createProgressIndicator() {
@@ -59,21 +60,21 @@ public class VcsLogProgress {
public boolean isRunning() {
synchronized (myLock) {
return myRunningTasksCount > 0;
return !myRunningTasks.isEmpty();
}
}
private void started() {
private void started(@NotNull ProgressIndicator indicator) {
synchronized (myLock) {
myRunningTasksCount++;
if (myRunningTasksCount == 1) fireNotification(ProgressListener::progressStarted);
myRunningTasks.add(indicator);
if (myRunningTasks.size() == 1) fireNotification(ProgressListener::progressStarted);
}
}
private void stopped() {
private void stopped(@NotNull ProgressIndicator indicator) {
synchronized (myLock) {
myRunningTasksCount--;
if (myRunningTasksCount == 0) fireNotification(ProgressListener::progressStopped);
myRunningTasks.remove(indicator);
if (myRunningTasks.isEmpty()) fireNotification(ProgressListener::progressStopped);
}
}
@@ -84,17 +85,26 @@ public class VcsLogProgress {
}
}
@Override
public void dispose() {
synchronized (myLock) {
for (ProgressIndicator indicator : myRunningTasks) {
indicator.cancel();
}
}
}
private class VcsLogProgressIndicator extends AbstractProgressIndicatorBase {
@Override
public synchronized void start() {
super.start();
started();
started(this);
}
@Override
public synchronized void stop() {
super.stop();
stopped();
stopped(this);
}
}
@@ -15,11 +15,13 @@
*/
package com.intellij.vcs.log.data;
import com.intellij.openapi.Disposable;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.progress.ProgressIndicator;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.openapi.progress.Task;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Disposer;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.vcs.VcsException;
import com.intellij.openapi.vfs.VirtualFile;
@@ -65,7 +67,8 @@ public class VcsLogRefresherImpl implements VcsLogRefresher {
@NotNull TopCommitsCache topCommitsDetailsCache,
@NotNull Consumer<DataPack> dataPackUpdateHandler,
@NotNull Consumer<Exception> exceptionHandler,
int recentCommitsCount) {
int recentCommitsCount,
@NotNull Disposable parentDisposable) {
myProject = project;
myHashMap = hashMap;
myProviders = providers;
@@ -74,6 +77,7 @@ public class VcsLogRefresherImpl implements VcsLogRefresher {
myExceptionHandler = exceptionHandler;
myRecentCommitCount = recentCommitsCount;
myProgress = new VcsLogProgress();
Disposer.register(parentDisposable, myProgress);
mySingleTaskController = new SingleTaskController<RefreshRequest, DataPack>(dataPack -> {
myDataPack = dataPack;
@@ -17,6 +17,7 @@ package com.intellij.vcs.log.data;
import com.intellij.openapi.Disposable;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.progress.ProcessCanceledException;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Condition;
import com.intellij.openapi.util.Disposer;
@@ -60,6 +61,7 @@ public class VcsLogStorageImpl implements Disposable, VcsLogStorage {
@NotNull private final PersistentEnumeratorBase<CommitId> myCommitIdEnumerator;
@NotNull private final PersistentEnumeratorBase<VcsRef> myRefsEnumerator;
@NotNull private final Consumer<Exception> myExceptionReporter;
private volatile boolean myDisposed = false;
public VcsLogStorageImpl(@NotNull Project project,
@NotNull Map<VirtualFile, VcsLogProvider> logProviders,
@@ -94,6 +96,7 @@ public class VcsLogStorageImpl implements Disposable, VcsLogStorage {
@Override
public int getCommitIndex(@NotNull Hash hash, @NotNull VirtualFile root) {
checkDisposed();
try {
return getOrPut(hash, root);
}
@@ -106,6 +109,7 @@ public class VcsLogStorageImpl implements Disposable, VcsLogStorage {
@Override
@Nullable
public CommitId getCommitId(int commitIndex) {
checkDisposed();
try {
CommitId commitId = doGetCommitId(commitIndex);
if (commitId == null) {
@@ -122,6 +126,7 @@ public class VcsLogStorageImpl implements Disposable, VcsLogStorage {
@Override
@Nullable
public CommitId findCommitId(@NotNull final Condition<CommitId> condition) {
checkDisposed();
try {
final Ref<CommitId> hashRef = Ref.create();
myCommitIdEnumerator.iterateData(new CommonProcessors.FindProcessor<CommitId>() {
@@ -144,6 +149,7 @@ public class VcsLogStorageImpl implements Disposable, VcsLogStorage {
@Override
public int getRefIndex(@NotNull VcsRef ref) {
checkDisposed();
try {
return myRefsEnumerator.enumerate(ref);
}
@@ -156,6 +162,7 @@ public class VcsLogStorageImpl implements Disposable, VcsLogStorage {
@Nullable
@Override
public VcsRef getVcsRef(int refIndex) {
checkDisposed();
try {
return myRefsEnumerator.valueOf(refIndex);
}
@@ -166,6 +173,7 @@ public class VcsLogStorageImpl implements Disposable, VcsLogStorage {
}
public void flush() {
checkDisposed();
myCommitIdEnumerator.force();
myRefsEnumerator.force();
}
@@ -173,6 +181,7 @@ public class VcsLogStorageImpl implements Disposable, VcsLogStorage {
@Override
public void dispose() {
try {
myDisposed = true;
myCommitIdEnumerator.close();
myRefsEnumerator.close();
}
@@ -181,6 +190,10 @@ public class VcsLogStorageImpl implements Disposable, VcsLogStorage {
}
}
private void checkDisposed() {
if (myDisposed) throw new ProcessCanceledException();
}
private static class MyCommitIdKeyDescriptor implements KeyDescriptor<CommitId> {
@NotNull private final List<VirtualFile> myRoots;
@NotNull private final TObjectIntHashMap<VirtualFile> myRootsReversed;
@@ -202,7 +202,8 @@ public class VcsLogRefresherTest extends VcsPlatformTest {
myLogData = new VcsLogData(myProject, myLogProviders, LOG::error);
Disposer.register(myProject, myLogData);
return new VcsLogRefresherImpl(myProject, myLogData.getHashMap(), myLogProviders, myLogData.getUserRegistry(),
myLogData.getTopCommitsCache(), dataPackConsumer, FAILING_EXCEPTION_HANDLER, RECENT_COMMITS_COUNT) {
myLogData.getTopCommitsCache(), dataPackConsumer, FAILING_EXCEPTION_HANDLER, RECENT_COMMITS_COUNT,
myLogData) {
@Override
protected void startNewBackgroundTask(@NotNull final Task.Backgroundable refreshTask) {
LOG.debug("Starting a background task...");