Skip to content

Commit

Permalink
Support [package-]private init/destroy methods in AOT mode
Browse files Browse the repository at this point in the history
Prior to this commit, private (and non-visible package-private)
init/destroy methods were not supported in AOT mode. The reason is that
such methods are tracked using their fully-qualified method names, and
the AOT support for init/destroy method previously did not take
fully-qualified method names into account. In addition, the invocation
order of init/destroy methods differed between standard JVM mode and
AOT mode.

This commit addresses these issues in the following ways.

- AbstractAutowireCapableBeanFactory.invokeCustomInitMethod(),
  DisposableBeanAdapter.determineDestroyMethod(), and
  BeanDefinitionPropertiesCodeGenerator.addInitDestroyHint() now parse
  fully-qualified method names to locate the correct init/destroy
  methods.

- AbstractAutowireCapableBeanFactory and DisposableBeanAdapter delegate
  to a new InitDestroyMethodUtils class which encapsulates the parsing
  of fully-qualified method names; however,
  BeanDefinitionPropertiesCodeGenerator duplicates this logic since it
  resides in a different package, and we do not currently want to make
  the functionality in InitDestroyMethodUtils public.

- Init/destroy methods detected via annotations (such as @PostConstruct
  and @PreDestroy) are now invoked prior to init/destroy methods that
  are explicitly configured by name or convention. This aligns with the
  invocation order in standard JVM mode; however,
  InitializingBean#afterPropertiesSet() and DisposableBean#destroy()
  are still invoked before annotated init/destroy methods in AOT mode
  which differs from standard JVM mode.

- Unit and integration tests have been updated to test the revised
  behavior.

Closes gh-30692
  • Loading branch information
sbrannen committed Jun 22, 2023
1 parent 32f061a commit ee6f7b1
Show file tree
Hide file tree
Showing 9 changed files with 266 additions and 28 deletions.
Expand Up @@ -182,7 +182,7 @@ private LifecycleMetadata findLifecycleMetadata(RootBeanDefinition beanDefinitio
private static String[] safeMerge(@Nullable String[] existingNames, Collection<LifecycleMethod> detectedMethods) {
Stream<String> detectedNames = detectedMethods.stream().map(LifecycleMethod::getIdentifier);
Stream<String> mergedNames = (existingNames != null ?
Stream.concat(Stream.of(existingNames), detectedNames) : detectedNames);
Stream.concat(detectedNames, Stream.of(existingNames)) : detectedNames);
return mergedNames.distinct().toArray(String[]::new);
}

Expand Down
Expand Up @@ -72,6 +72,7 @@
*
* @author Phillip Webb
* @author Stephane Nicoll
* @author Sam Brannen
* @since 6.0
*/
class BeanDefinitionPropertiesCodeGenerator {
Expand Down Expand Up @@ -138,7 +139,25 @@ private void addInitDestroyMethods(Builder code, AbstractBeanDefinition beanDefi
}

private void addInitDestroyHint(Class<?> beanUserClass, String methodName) {
Method method = ReflectionUtils.findMethod(beanUserClass, methodName);
Class<?> methodDeclaringClass = beanUserClass;

// Parse fully-qualified method name if necessary.
int indexOfDot = methodName.lastIndexOf('.');
if (indexOfDot > 0) {
String className = methodName.substring(0, indexOfDot);
methodName = methodName.substring(indexOfDot + 1);
if (!beanUserClass.getName().equals(className)) {
try {
methodDeclaringClass = ClassUtils.forName(className, beanUserClass.getClassLoader());
}
catch (Throwable ex) {
throw new IllegalStateException("Failed to load Class [" + className +
"] from ClassLoader [" + beanUserClass.getClassLoader() + "]", ex);
}
}
}

Method method = ReflectionUtils.findMethod(methodDeclaringClass, methodName);
if (method != null) {
this.hints.reflection().registerMethod(method, ExecutableMode.INVOKE);
}
Expand Down
Expand Up @@ -66,6 +66,7 @@
import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessor;
import org.springframework.beans.factory.config.SmartInstantiationAwareBeanPostProcessor;
import org.springframework.beans.factory.config.TypedStringValue;
import org.springframework.beans.factory.support.InitDestroyMethodUtils.MethodDescriptor;
import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.MethodParameter;
import org.springframework.core.NamedThreadLocal;
Expand Down Expand Up @@ -1841,18 +1842,23 @@ protected void invokeInitMethods(String beanName, Object bean, @Nullable RootBea
protected void invokeCustomInitMethod(String beanName, Object bean, RootBeanDefinition mbd, String initMethodName)
throws Throwable {

Class<?> beanClass = bean.getClass();
MethodDescriptor descriptor =
InitDestroyMethodUtils.createMethodDescriptor(beanName, beanClass, initMethodName);
String methodName = descriptor.methodName();

Method initMethod = (mbd.isNonPublicAccessAllowed() ?
BeanUtils.findMethod(bean.getClass(), initMethodName) :
ClassUtils.getMethodIfAvailable(bean.getClass(), initMethodName));
BeanUtils.findMethod(descriptor.declaringClass(), methodName) :
ClassUtils.getMethodIfAvailable(beanClass, methodName));

if (initMethod == null) {
if (mbd.isEnforceInitMethod()) {
throw new BeanDefinitionValidationException("Could not find an init method named '" +
initMethodName + "' on bean with name '" + beanName + "'");
methodName + "' on bean with name '" + beanName + "'");
}
else {
if (logger.isTraceEnabled()) {
logger.trace("No default init method named '" + initMethodName +
logger.trace("No default init method named '" + methodName +
"' found on bean with name '" + beanName + "'");
}
// Ignore non-existent default lifecycle methods.
Expand All @@ -1861,9 +1867,9 @@ protected void invokeCustomInitMethod(String beanName, Object bean, RootBeanDefi
}

if (logger.isTraceEnabled()) {
logger.trace("Invoking init method '" + initMethodName + "' on bean with name '" + beanName + "'");
logger.trace("Invoking init method '" + methodName + "' on bean with name '" + beanName + "'");
}
Method methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(initMethod, bean.getClass());
Method methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(initMethod, beanClass);

try {
ReflectionUtils.makeAccessible(methodToInvoke);
Expand Down
Expand Up @@ -28,6 +28,7 @@
import org.springframework.beans.BeanUtils;
import org.springframework.beans.factory.DisposableBean;
import org.springframework.beans.factory.config.DestructionAwareBeanPostProcessor;
import org.springframework.beans.factory.support.InitDestroyMethodUtils.MethodDescriptor;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
Expand Down Expand Up @@ -255,12 +256,16 @@ else if (this.destroyMethodNames != null) {
private Method determineDestroyMethod(String destroyMethodName) {
try {
Class<?> beanClass = this.bean.getClass();
Method destroyMethod = findDestroyMethod(beanClass, destroyMethodName);
MethodDescriptor descriptor =
InitDestroyMethodUtils.createMethodDescriptor(this.beanName, beanClass, destroyMethodName);
String methodName = descriptor.methodName();

Method destroyMethod = findDestroyMethod(descriptor.declaringClass(), methodName);
if (destroyMethod != null) {
return destroyMethod;
}
for (Class<?> beanInterface : beanClass.getInterfaces()) {
destroyMethod = findDestroyMethod(beanInterface, destroyMethodName);
destroyMethod = findDestroyMethod(beanInterface, methodName);
if (destroyMethod != null) {
return destroyMethod;
}
Expand Down
@@ -0,0 +1,83 @@
/*
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.beans.factory.support;

import org.springframework.util.ClassUtils;

/**
* Internal utilities for working with init/destroy methods.
*
* @author Sam Brannen
* @since 6.0.11
*/
abstract class InitDestroyMethodUtils {

/**
* Create a {@link MethodDescriptor} for the supplied bean class and method
* name.
* <p>The supplied {@code methodName} may be a {@linkplain Method#getName()
* simple method name} or a
* {@linkplain org.springframework.util.ClassUtils#getQualifiedMethodName(Method)
* qualified method name} for package-private and {@code private} methods.
* <p>If the method name is fully qualified, this utility will parse the
* method name and its declaring class from the qualified method and name
* and then attempt to load the method's declaring class using the
* {@link ClassLoader} of the supplied {@code beanClass}. Otherwise, the
* returned descriptor will reference the supplied {@code beanClass} and
* {@code methodName}.
* @param beanName the bean name in the factory (for debugging purposes)
* @param beanClass the bean class
* @param methodName the name of the method
* @return a new {@code MethodDescriptor}; never {@code null}
*/
static MethodDescriptor createMethodDescriptor(String beanName, Class<?> beanClass, String methodName) {
try {
Class<?> declaringClass = beanClass;
String methodNameToUse = methodName;

// Parse fully-qualified method name if necessary.
int indexOfDot = methodName.lastIndexOf('.');
if (indexOfDot > 0) {
String className = methodName.substring(0, indexOfDot);
methodNameToUse = methodName.substring(indexOfDot + 1);
if (!beanClass.getName().equals(className)) {
declaringClass = ClassUtils.forName(className, beanClass.getClassLoader());
}
}
return new MethodDescriptor(declaringClass, methodNameToUse);
}
catch (ClassNotFoundException | LinkageError | IllegalArgumentException ex) {
throw new BeanDefinitionValidationException(
"Could not create descriptor for method '%s' on bean with name '%s': %s"
.formatted(methodName, beanName, ex.getMessage()));
}
}


/**
* Descriptor for a {@link java.lang.reflect.Method Method} which holds
* a reference to the method's {@linkplain Class declaring class}, the
* name of the method, and the method's parameter types.
*
* @param declaringClass the method's declaring class
* @param methodName the name of the method
* @param parameterTypes the types or parameters accepted by the method
*/
record MethodDescriptor(Class<?> declaringClass, String methodName, Class<?>... parameterTypes) {
}

}
Expand Up @@ -70,8 +70,8 @@ void processAheadOfTimeWhenHasInitDestroyAnnotationsAndCustomDefinedMethodNamesA
beanDefinition.setDestroyMethodNames("customDestroyMethod");
processAheadOfTime(beanDefinition);
RootBeanDefinition mergedBeanDefinition = getMergedBeanDefinition();
assertThat(mergedBeanDefinition.getInitMethodNames()).containsExactly("customInitMethod", "initMethod");
assertThat(mergedBeanDefinition.getDestroyMethodNames()).containsExactly("customDestroyMethod", "destroyMethod");
assertThat(mergedBeanDefinition.getInitMethodNames()).containsExactly("initMethod", "customInitMethod");
assertThat(mergedBeanDefinition.getDestroyMethodNames()).containsExactly("destroyMethod", "customDestroyMethod");
}

@Test
Expand Down Expand Up @@ -129,16 +129,16 @@ void processAheadOfTimeWithMultipleLevelsOfPublicAndPrivateInitAndDestroyMethods
RootBeanDefinition mergedBeanDefinition = getMergedBeanDefinition();
assertSoftly(softly -> {
softly.assertThat(mergedBeanDefinition.getInitMethodNames()).containsExactly(
"afterPropertiesSet",
"customInit",
CustomAnnotatedPrivateInitDestroyBean.class.getName() + ".privateInit", // fully-qualified private method
CustomAnnotatedPrivateSameNameInitDestroyBean.class.getName() + ".privateInit" // fully-qualified private method
CustomAnnotatedPrivateSameNameInitDestroyBean.class.getName() + ".privateInit", // fully-qualified private method
"afterPropertiesSet",
"customInit"
);
softly.assertThat(mergedBeanDefinition.getDestroyMethodNames()).containsExactly(
"destroy",
"customDestroy",
CustomAnnotatedPrivateSameNameInitDestroyBean.class.getName() + ".privateDestroy", // fully-qualified private method
CustomAnnotatedPrivateInitDestroyBean.class.getName() + ".privateDestroy" // fully-qualified private method
CustomAnnotatedPrivateInitDestroyBean.class.getName() + ".privateDestroy", // fully-qualified private method
"destroy",
"customDestroy"
);
});
}
Expand Down
Expand Up @@ -376,6 +376,9 @@ void multipleItems() {
@Nested
class InitDestroyMethodTests {

private final String privateInitMethod = InitDestroyBean.class.getName() + ".privateInit";
private final String privateDestroyMethod = InitDestroyBean.class.getName() + ".privateDestroy";

@BeforeEach
void setTargetType() {
beanDefinition.setTargetType(InitDestroyBean.class);
Expand All @@ -393,11 +396,18 @@ void singleInitMethod() {
assertHasMethodInvokeHints(InitDestroyBean.class, "init");
}

@Test
void privateInitMethod() {
beanDefinition.setInitMethodName(privateInitMethod);
compile((beanDef, compiled) -> assertThat(beanDef.getInitMethodNames()).containsExactly(privateInitMethod));
assertHasMethodInvokeHints(InitDestroyBean.class, "privateInit");
}

@Test
void multipleInitMethods() {
beanDefinition.setInitMethodNames("init", "init2");
compile((beanDef, compiled) -> assertThat(beanDef.getInitMethodNames()).containsExactly("init", "init2"));
assertHasMethodInvokeHints(InitDestroyBean.class, "init", "init2");
beanDefinition.setInitMethodNames("init", privateInitMethod);
compile((beanDef, compiled) -> assertThat(beanDef.getInitMethodNames()).containsExactly("init", privateInitMethod));
assertHasMethodInvokeHints(InitDestroyBean.class, "init", "privateInit");
}

@Test
Expand All @@ -412,11 +422,18 @@ void singleDestroyMethod() {
assertHasMethodInvokeHints(InitDestroyBean.class, "destroy");
}

@Test
void privateDestroyMethod() {
beanDefinition.setDestroyMethodName(privateDestroyMethod);
compile((beanDef, compiled) -> assertThat(beanDef.getDestroyMethodNames()).containsExactly(privateDestroyMethod));
assertHasMethodInvokeHints(InitDestroyBean.class, "privateDestroy");
}

@Test
void multipleDestroyMethods() {
beanDefinition.setDestroyMethodNames("destroy", "destroy2");
compile((beanDef, compiled) -> assertThat(beanDef.getDestroyMethodNames()).containsExactly("destroy", "destroy2"));
assertHasMethodInvokeHints(InitDestroyBean.class, "destroy", "destroy2");
beanDefinition.setDestroyMethodNames("destroy", privateDestroyMethod);
compile((beanDef, compiled) -> assertThat(beanDef.getDestroyMethodNames()).containsExactly("destroy", privateDestroyMethod));
assertHasMethodInvokeHints(InitDestroyBean.class, "destroy", "privateDestroy");
}

}
Expand Down Expand Up @@ -461,13 +478,15 @@ static class InitDestroyBean {
void init() {
}

void init2() {
@SuppressWarnings("unused")
private void privateInit() {
}

void destroy() {
}

void destroy2() {
@SuppressWarnings("unused")
private void privateDestroy() {
}

}
Expand Down

0 comments on commit ee6f7b1

Please sign in to comment.