Skip to content

Commit

Permalink
Handle all errors and runtime exceptions that may be thrown
Browse files Browse the repository at this point in the history
We can fail to regain our lock in other cases than just
operation cancelled exception, so capture all of those
cases to throw an FailedToReAcquireLockException.

Also, fix another finally block that assumes it has the lock.

Fixes #128
  • Loading branch information
jonahgraham committed Nov 9, 2022
1 parent a3a6682 commit cc67bd3
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,26 @@
/**
* This exception is raised when {@link YieldableIndexLock#yield()} fails to
* reacquire the lock after yielding, this may be due to an {@link InterruptedException}
* or an {@link OperationCanceledException} which will be the nested exception.
* or an {@link OperationCanceledException} or some other type of runtime exception or
* error (especially assertion errors) which will be the nested exception.
*/
public class FailedToReAcquireLockException extends Exception {

public FailedToReAcquireLockException(InterruptedException e) {
super(e);
Assert.isNotNull(e);
public FailedToReAcquireLockException(Throwable t) {
super(t);
Assert.isNotNull(t);
}

public FailedToReAcquireLockException(OperationCanceledException e) {
super(e);
Assert.isNotNull(e);
}

public void reThrow() throws InterruptedException, OperationCanceledException {
public void reThrow() throws InterruptedException {
if (getCause() instanceof InterruptedException ie) {
throw ie;
}
if (getCause() instanceof OperationCanceledException oce) {
throw oce;
if (getCause() instanceof RuntimeException re) {
throw re;
}
if (getCause() instanceof Error er) {
throw er;
}
throw new RuntimeException("Unexpectedly the exception cause was none of the allowed types", this); //$NON-NLS-1$
throw new RuntimeException("Checked exception changed wrapped in runtime exception", getCause()); //$NON-NLS-1$
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ private IIndexFragmentFile storeFileInIndex(Data data, FileInAST astFile, int st
ISignificantMacros significantMacros = fileKey.getSignificantMacros();
IIndexFragmentFile oldFile = index.getWritableFile(storageLinkageID, location, significantMacros);
file = index.addUncommittedFile(storageLinkageID, location, significantMacros);
boolean hasLock = true;
try {
boolean pragmaOnce = owner != null ? owner.hasPragmaOnceSemantics() : data.fAST.hasPragmaOnceSemantics();
file.setPragmaOnceSemantics(pragmaOnce);
Expand Down Expand Up @@ -695,8 +696,21 @@ private IIndexFragmentFile storeFileInIndex(Data data, FileInAST astFile, int st
file.setSizeAndEncodingHashcode(computeFileSizeAndEncodingHashcode(astFile.fileSize, location));
file.setContentsHash(astFile.contentsHash);
file = index.commitUncommittedFile();
} catch (FailedToReAcquireLockException e) {
hasLock = false;
throw e;
} finally {
index.clearUncommittedFile();
try {
index.clearUncommittedFile();
} catch (Throwable e) {
// Without the lock we need to still do some cleanup, but some
// of it will fail with an assertion error when running unit tests
// so to not lose the original exception, we suppress it if
// we don't have the lock.
if (hasLock) {
throw e;
}
}
}
return file;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import org.eclipse.cdt.internal.core.index.IWritableIndex;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.OperationCanceledException;

/**
* Write lock on the index that can be yielded temporarily to unblock threads that need
Expand Down Expand Up @@ -67,10 +66,8 @@ public void yield() throws FailedToReAcquireLockException {
lastLockTime = 0;
try {
acquire();
} catch (OperationCanceledException e) {
throw new FailedToReAcquireLockException(e);
} catch (InterruptedException e) {
throw new FailedToReAcquireLockException(e);
} catch (Throwable t) {
throw new FailedToReAcquireLockException(t);
}
}
}
Expand Down

0 comments on commit cc67bd3

Please sign in to comment.