Skip to content

Commit ca1bf5f

Browse files
committed
fix: Make TypeAdaptor method adaptation threadsafe
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
1 parent f2077c5 commit ca1bf5f

File tree

1 file changed

+18
-8
lines changed

1 file changed

+18
-8
lines changed

src/main/java/spoon/support/adaption/TypeAdaptor.java

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
import spoon.SpoonException;
1111
import spoon.processing.FactoryAccessor;
12-
import spoon.reflect.code.CtBlock;
1312
import spoon.reflect.declaration.CtConstructor;
1413
import spoon.reflect.declaration.CtElement;
1514
import spoon.reflect.declaration.CtExecutable;
@@ -249,12 +248,27 @@ private static boolean supertypeReachableInInheritanceTree(CtType<?> base, Strin
249248
* @return the input method but with the return type, parameter types and thrown types adapted to
250249
* the context of this type adapter
251250
*/
252-
@SuppressWarnings("unchecked")
253251
public CtMethod<?> adaptMethod(CtMethod<?> inputMethod) {
252+
return adaptMethod(inputMethod, true);
253+
}
254+
255+
@SuppressWarnings("unchecked")
256+
private CtMethod<?> adaptMethod(CtMethod<?> inputMethod, boolean cloneBody) {
254257
if (useLegacyTypeAdaption(inputMethod)) {
255258
return legacyAdaptMethod(inputMethod);
256259
}
257-
CtMethod<?> clonedMethod = inputMethod.clone();
260+
CtMethod<?> clonedMethod;
261+
if (cloneBody) {
262+
clonedMethod = inputMethod.clone();
263+
} else {
264+
clonedMethod = inputMethod.getFactory().createMethod().setSimpleName(inputMethod.getSimpleName());
265+
for (CtParameter<?> parameter : inputMethod.getParameters()) {
266+
clonedMethod.addParameter(parameter.clone());
267+
}
268+
for (CtTypeParameter parameter : inputMethod.getFormalCtTypeParameters()) {
269+
clonedMethod.addFormalCtTypeParameter(parameter.clone());
270+
}
271+
}
258272

259273
for (int i = 0; i < clonedMethod.getFormalCtTypeParameters().size(); i++) {
260274
CtTypeParameter clonedParameter = clonedMethod.getFormalCtTypeParameters().get(i);
@@ -450,13 +464,9 @@ public boolean isOverriding(CtMethod<?> subMethod, CtMethod<?> superMethod) {
450464
if (!isSubtype(subDeclaringType, superDeclaringType.getReference())) {
451465
return false;
452466
}
453-
454467
// We don't need to clone the body here, so leave it out
455-
CtBlock<?> superBody = superMethod.getBody();
456-
superMethod.setBody(null);
457468
CtMethod<?> adapted = new TypeAdaptor(subMethod.getDeclaringType())
458-
.adaptMethod(superMethod);
459-
superMethod.setBody(superBody);
469+
.adaptMethod(superMethod, false);
460470

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

0 commit comments

Comments
 (0)