1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-06-08 02:06:01 +02:00

Stop trying to clear result caches when lock isn't held

The indexer has a feature that allows readers of the index
to read the index in the middle of write operations. This
is done by using a YeildableIndexLock.

The YeildableIndexLock's yield method can be called to
temporarily give up the write lock. However the assumption
in the code was that it would always successfully
reaquire the lock after that.

However, if the indexing was cancelled the lock would
fail to be reaquired. Therefore the code that thinks it
owns the lock no longer owns it. In this case the code
in PDOMWriter.storeSymbolsInIndex's finally block.

Therefore I have added an new exception type to explicitly
identify this use case so the original code can differentiate
between cases where an exception was thrown where the lock
is still held, and cases where the lock is no longer held.

Note that instead of a new exception caught like this:

```java
} catch (FailedToReAcquireLockException e) {
    hasLock = false;
    e.reThrow();
```

I could have done this:

```java
} catch (InterruptedException | OperationCanceledException e) {
    hasLock = false;
    throw e;
```

But it is not obvious that nothing else other than the
acquire can raise an OperationCanceledException because it
is a RuntimeException. By having a new checked exception we
can know for sure that in the finally block we have lost
our lock.

There are no API implications of this change as all the classes
and interfaces are internal to CDT.

Fixes #128
This commit is contained in:
Jonah Graham 2022-11-07 11:40:14 -05:00
parent 1b080ac87e
commit 571d62d6f9
8 changed files with 81 additions and 16 deletions

View file

@ -22,6 +22,7 @@ import org.eclipse.cdt.core.index.IIndex;
import org.eclipse.cdt.core.index.IIndexFileLocation;
import org.eclipse.cdt.core.parser.ISignificantMacros;
import org.eclipse.cdt.internal.core.pdom.ASTFilePathResolver;
import org.eclipse.cdt.internal.core.pdom.FailedToReAcquireLockException;
import org.eclipse.cdt.internal.core.pdom.YieldableIndexLock;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
@ -116,7 +117,7 @@ public interface IWritableIndex extends IIndex {
*/
void setFileContent(IIndexFragmentFile sourceFile, int linkageID, IncludeInformation[] includes,
IASTPreprocessorStatement[] macros, IASTName[][] names, ASTFilePathResolver resolver,
YieldableIndexLock lock) throws CoreException, InterruptedException;
YieldableIndexLock lock) throws CoreException, FailedToReAcquireLockException;
/**
* Clears the entire index.

View file

@ -21,6 +21,7 @@ import org.eclipse.cdt.core.index.IIndexFileLocation;
import org.eclipse.cdt.core.parser.ISignificantMacros;
import org.eclipse.cdt.internal.core.index.IWritableIndex.IncludeInformation;
import org.eclipse.cdt.internal.core.pdom.ASTFilePathResolver;
import org.eclipse.cdt.internal.core.pdom.FailedToReAcquireLockException;
import org.eclipse.cdt.internal.core.pdom.YieldableIndexLock;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
@ -84,7 +85,7 @@ public interface IWritableIndexFragment extends IIndexFragment {
*/
void addFileContent(IIndexFragmentFile sourceFile, IncludeInformation[] includes,
IASTPreprocessorStatement[] macros, IASTName[][] names, ASTFilePathResolver resolver,
YieldableIndexLock lock) throws CoreException, InterruptedException;
YieldableIndexLock lock) throws CoreException, FailedToReAcquireLockException;
/**
* Acquires a write lock, while giving up a certain amount of read locks.

View file

@ -20,6 +20,7 @@ import org.eclipse.cdt.core.dom.ast.IASTPreprocessorStatement;
import org.eclipse.cdt.core.index.IIndexFileLocation;
import org.eclipse.cdt.core.parser.ISignificantMacros;
import org.eclipse.cdt.internal.core.pdom.ASTFilePathResolver;
import org.eclipse.cdt.internal.core.pdom.FailedToReAcquireLockException;
import org.eclipse.cdt.internal.core.pdom.YieldableIndexLock;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
@ -78,7 +79,7 @@ public class WritableCIndex extends CIndex implements IWritableIndex {
@Override
public void setFileContent(IIndexFragmentFile file, int linkageID, IncludeInformation[] includes,
IASTPreprocessorStatement[] macros, IASTName[][] names, ASTFilePathResolver resolver,
YieldableIndexLock lock) throws CoreException, InterruptedException {
YieldableIndexLock lock) throws CoreException, FailedToReAcquireLockException {
assert getWritableFragment() == file.getIndexFragment();
for (IncludeInformation include : includes) {

View file

@ -0,0 +1,42 @@
/*******************************************************************************
* Copyright (c) 2022 Kichwa Coders Canada, Inc. and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*******************************************************************************/
package org.eclipse.cdt.internal.core.pdom;
import org.eclipse.core.runtime.Assert;
import org.eclipse.core.runtime.OperationCanceledException;
/**
* 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.
*/
public class FailedToReAcquireLockException extends Exception {
public FailedToReAcquireLockException(InterruptedException e) {
super(e);
Assert.isNotNull(e);
}
public FailedToReAcquireLockException(OperationCanceledException e) {
super(e);
Assert.isNotNull(e);
}
public void reThrow() throws InterruptedException, OperationCanceledException {
if (getCause() instanceof InterruptedException ie) {
throw ie;
}
if (getCause() instanceof OperationCanceledException oce) {
throw oce;
}
throw new RuntimeException("Unexpectedly the exception cause was none of the allowed types", this); //$NON-NLS-1$
}
}

View file

@ -322,6 +322,7 @@ public abstract class PDOMWriter implements IPDOMASTProcessor {
Throwable th = null;
YieldableIndexLock lock = new YieldableIndexLock(data.fIndex, false, progress.split(1));
lock.acquire();
boolean hasLock = true;
try {
final boolean isReplacement = ctx != null && fileInAST.includeStatement == null;
IIndexFragmentFile ifile = null;
@ -342,11 +343,21 @@ public abstract class PDOMWriter implements IPDOMASTProcessor {
}
}
}
} catch (FailedToReAcquireLockException e) {
hasLock = false;
e.reThrow();
} catch (OperationCanceledException e) {
throw e;
} catch (RuntimeException | StackOverflowError | AssertionError e) {
th = e;
} finally {
// If we failed to reacquire the yieldable index lock after yielding (for example
// because the project got closed) then FailedToReAcquireLockException will
// have been thrown and we are done here.
//
// Note - we can't ask the index if it is locked because someone else may have now
// locked it once we failed to reacquire the lock.
if (hasLock) {
// Because the caller holds a read-lock, the result cache of the index is never cleared.
// Before releasing the lock for the last time in this AST, we clear the result cache.
if (i == data.fSelectedFiles.length - 1) {
@ -354,6 +365,7 @@ public abstract class PDOMWriter implements IPDOMASTProcessor {
}
lock.release();
}
}
if (th != null) {
data.fStatuses.add(createStatus(NLS.bind(Messages.PDOMWriter_errorWhileParsing,
fileInAST.fileContentKey.getLocation().getURI().getPath()), th));
@ -619,7 +631,7 @@ public abstract class PDOMWriter implements IPDOMASTProcessor {
}
private IIndexFragmentFile storeFileInIndex(Data data, FileInAST astFile, int storageLinkageID,
YieldableIndexLock lock, IProgressMonitor monitor) throws CoreException, InterruptedException {
YieldableIndexLock lock, IProgressMonitor monitor) throws CoreException, FailedToReAcquireLockException {
final IWritableIndex index = data.fIndex;
IIndexFragmentFile file;
// We create a temporary PDOMFile with zero timestamp, add names to it, then replace

View file

@ -147,7 +147,7 @@ public class WritablePDOM extends PDOM implements IWritableIndexFragment {
@Override
public void addFileContent(IIndexFragmentFile sourceFile, IncludeInformation[] includes,
IASTPreprocessorStatement[] macros, IASTName[][] names, ASTFilePathResolver pathResolver,
YieldableIndexLock lock) throws CoreException, InterruptedException {
YieldableIndexLock lock) throws CoreException, FailedToReAcquireLockException {
assert sourceFile.getIndexFragment() == this;
PDOMFile pdomFile = (PDOMFile) sourceFile;

View file

@ -15,6 +15,7 @@ package org.eclipse.cdt.internal.core.pdom;
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
@ -56,16 +57,21 @@ public class YieldableIndexLock {
}
/**
* Yields the lock temporarily if it was held for YIELD_INTERVAL or more, and somebody is waiting
* for a read lock.
* @throws InterruptedException
* Yields the lock temporarily if somebody is waiting for a read lock.
* @throws FailedToReAcquireLockException when lock is not reacquired.
*/
public void yield() throws InterruptedException {
public void yield() throws FailedToReAcquireLockException {
if (index.hasWaitingReaders()) {
index.releaseWriteLock(false);
cumulativeLockTime += System.currentTimeMillis() - lastLockTime;
lastLockTime = 0;
try {
acquire();
} catch (OperationCanceledException e) {
throw new FailedToReAcquireLockException(e);
} catch (InterruptedException e) {
throw new FailedToReAcquireLockException(e);
}
}
}

View file

@ -47,6 +47,7 @@ import org.eclipse.cdt.internal.core.index.IWritableIndex.IncludeInformation;
import org.eclipse.cdt.internal.core.index.IWritableIndexFragment;
import org.eclipse.cdt.internal.core.index.IndexFileLocation;
import org.eclipse.cdt.internal.core.parser.scanner.SignificantMacros;
import org.eclipse.cdt.internal.core.pdom.FailedToReAcquireLockException;
import org.eclipse.cdt.internal.core.pdom.PDOM;
import org.eclipse.cdt.internal.core.pdom.YieldableIndexLock;
import org.eclipse.cdt.internal.core.pdom.db.BTree;
@ -472,7 +473,8 @@ public class PDOMFile implements IIndexFragmentFile {
return fLinkage;
}
public void addNames(IASTName[][] names, YieldableIndexLock lock) throws CoreException, InterruptedException {
public void addNames(IASTName[][] names, YieldableIndexLock lock)
throws CoreException, FailedToReAcquireLockException {
assert getFirstName() == null;
assert getFirstMacroReference() == null;
final PDOMLinkage linkage = getLinkage();