Skip to content

Commit

Permalink
fix: Make TypeAdaptor method adaptation thread safe (#5621)
Browse files Browse the repository at this point in the history
When checking whether a method overrides another, we need to adapt both
methods to a common ground.
In an early version this required cloning the entire method, including
its body, to perform the adaption. PR #4862 fixed this by nulling the
body before cloning the method and then restoring it afterwards. This
operation is not thread safe, as it may write concurrently and also
modifies what other parallel threads might see when traversing the
model.

This patch now introduces a private override specifying whether we are
only interested in the signature. If that is the case, we explicitly
construct a new method using the factory instead of cloning the
original.

Closes: #5619

Co-authored-by: I-Al-Istannen <[email protected]>
  • Loading branch information
MartinWitt and I-Al-Istannen authored Jan 22, 2024
1 parent 0008076 commit 7121a4c
Showing 1 changed file with 18 additions and 8 deletions.
26 changes: 18 additions & 8 deletions src/main/java/spoon/support/adaption/TypeAdaptor.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import spoon.SpoonException;
import spoon.processing.FactoryAccessor;
import spoon.reflect.code.CtBlock;
import spoon.reflect.declaration.CtConstructor;
import spoon.reflect.declaration.CtElement;
import spoon.reflect.declaration.CtExecutable;
Expand Down Expand Up @@ -249,12 +248,27 @@ private static boolean supertypeReachableInInheritanceTree(CtType<?> base, Strin
* @return the input method but with the return type, parameter types and thrown types adapted to
* the context of this type adapter
*/
@SuppressWarnings("unchecked")
public CtMethod<?> adaptMethod(CtMethod<?> inputMethod) {
return adaptMethod(inputMethod, true);
}

@SuppressWarnings("unchecked")
private CtMethod<?> adaptMethod(CtMethod<?> inputMethod, boolean cloneBody) {
if (useLegacyTypeAdaption(inputMethod)) {
return legacyAdaptMethod(inputMethod);
}
CtMethod<?> clonedMethod = inputMethod.clone();
CtMethod<?> clonedMethod;
if (cloneBody) {
clonedMethod = inputMethod.clone();
} else {
clonedMethod = inputMethod.getFactory().createMethod().setSimpleName(inputMethod.getSimpleName());
for (CtParameter<?> parameter : inputMethod.getParameters()) {
clonedMethod.addParameter(parameter.clone());
}
for (CtTypeParameter parameter : inputMethod.getFormalCtTypeParameters()) {
clonedMethod.addFormalCtTypeParameter(parameter.clone());
}
}

for (int i = 0; i < clonedMethod.getFormalCtTypeParameters().size(); i++) {
CtTypeParameter clonedParameter = clonedMethod.getFormalCtTypeParameters().get(i);
Expand Down Expand Up @@ -450,13 +464,9 @@ public boolean isOverriding(CtMethod<?> subMethod, CtMethod<?> superMethod) {
if (!isSubtype(subDeclaringType, superDeclaringType.getReference())) {
return false;
}

// We don't need to clone the body here, so leave it out
CtBlock<?> superBody = superMethod.getBody();
superMethod.setBody(null);
CtMethod<?> adapted = new TypeAdaptor(subMethod.getDeclaringType())
.adaptMethod(superMethod);
superMethod.setBody(superBody);
.adaptMethod(superMethod, false);

for (int i = 0; i < subMethod.getParameters().size(); i++) {
CtParameter<?> subParam = subMethod.getParameters().get(i);
Expand Down

0 comments on commit 7121a4c

Please sign in to comment.