From 7121a4c27ef29896406c0efba6912fbe356d2684 Mon Sep 17 00:00:00 2001 From: Martin Wittlinger Date: Mon, 22 Jan 2024 20:04:17 +0100 Subject: [PATCH] fix: Make TypeAdaptor method adaptation thread safe (#5621) 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 --- .../spoon/support/adaption/TypeAdaptor.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/main/java/spoon/support/adaption/TypeAdaptor.java b/src/main/java/spoon/support/adaption/TypeAdaptor.java index 34bac74c489..dacd5de41bc 100644 --- a/src/main/java/spoon/support/adaption/TypeAdaptor.java +++ b/src/main/java/spoon/support/adaption/TypeAdaptor.java @@ -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; @@ -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); @@ -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);