From cc67bd3b5907b62239eb7bf6b88cb6253da689b1 Mon Sep 17 00:00:00 2001 From: Jonah Graham Date: Mon, 7 Nov 2022 21:08:32 -0500 Subject: [PATCH] Handle all errors and runtime exceptions that may be thrown 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 --- .../pdom/FailedToReAcquireLockException.java | 25 +++++++++---------- .../cdt/internal/core/pdom/PDOMWriter.java | 16 +++++++++++- .../core/pdom/YieldableIndexLock.java | 7 ++---- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/FailedToReAcquireLockException.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/FailedToReAcquireLockException.java index 4ffc1ccdeb8..978bb845e02 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/FailedToReAcquireLockException.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/FailedToReAcquireLockException.java @@ -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$ } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOMWriter.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOMWriter.java index 6f26581a1cd..29bcca3e418 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOMWriter.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOMWriter.java @@ -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); @@ -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; } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/YieldableIndexLock.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/YieldableIndexLock.java index 618ffd175c2..cbafbcb494a 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/YieldableIndexLock.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/YieldableIndexLock.java @@ -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 @@ -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); } } }