Skip to content
This repository has been archived by the owner on Feb 23, 2023. It is now read-only.

Reflection hint declared on bean definition type are not usable on bean implementation #1121

Closed
christophstrobl opened this issue Oct 1, 2021 · 4 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@christophstrobl
Copy link
Contributor

In case a destroy method is registered for an interface the actual invocation of that method on the concrete bean type fails due to missing reflection configuration for the implementation class.

This can be seen when shutting down the data-mongodb sample.

Bean name: "mongo",
Bean type: MongoClient (extends Closeable)

the resulting reflection configuration holds the entry for the client

{
  "name": "com.mongodb.client.MongoClient",
  "methods": [
    {
      "name": "close",
      "parameterTypes": []
    }
  ]
},

On shutown InitDestroyBeanPostProcessor receives the actual MongoClient implementation com.mongodb.client.internal.MongoClientImpl for which no reflection entry is available resulting in an NPE.

o.s.a.c.a.InitDestroyBeanPostProcessor   : Invoking destroy method on bean 'mongo': null
o.s.a.c.a.InitDestroyBeanPostProcessor   : Failed to invoke destroy method on bean with name 'mongo'

java.lang.NullPointerException: null
	at org.springframework.util.ReflectionUtils.makeAccessible(ReflectionUtils.java:571) ~[na:na]
	at org.springframework.aot.context.annotation.InitDestroyBeanPostProcessor.invokeMethod(InitDestroyBeanPostProcessor.java:121) ~[na:na]
	at org.springframework.aot.context.annotation.InitDestroyBeanPostProcessor.invokeDestroyMethod(InitDestroyBeanPostProcessor.java:95) ~[na:na]
	at org.springframework.aot.context.annotation.InitDestroyBeanPostProcessor.postProcessBeforeDestruction(InitDestroyBeanPostProcessor.java:85) ~[na:na]
	at org.springframework.beans.factory.support.DisposableBeanAdapter.destroy(DisposableBeanAdapter.java:184) ~[na:na]
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.destroyBean(DefaultSingletonBeanRegistry.java:587) ~[na:na]

If we keep track of the type used in the bean definition we could lookup the method on that one and do the invocation on the instance which works fine when I gave it a spin.

o.s.a.c.a.InitDestroyBeanPostProcessor   : Invoking destroy method on bean 'mongo': public abstract void com.mongodb.client.MongoClient.close()
@sdeleuze sdeleuze added this to the 0.11.0-M1 milestone Oct 1, 2021
@sdeleuze sdeleuze added the type: bug A general bug label Oct 1, 2021
@snicoll snicoll self-assigned this Oct 1, 2021
@snicoll
Copy link
Contributor

snicoll commented Oct 1, 2021

It turns out that we're doing more work than we should. DisposableBeanAdapter is already taking care of AutoCloseable and friends so there's no need to process (inferred) destroy method name.

However, we need to process them for reflection configuration purposes so we'll have to retain that information somehow.

@snicoll
Copy link
Contributor

snicoll commented Oct 1, 2021

And I take that back. DisposableBeanAdapter does this based on the fact the BeanDefinition has an inferred destroy method name but we're not writing that back since we're handling it ourselves (discovering the method to call at runtime).

@snicoll snicoll changed the title NullPointerException invoking destroy method. Reflection hint declared on bean definition type are not usable on bean implementation Oct 1, 2021
@snicoll
Copy link
Contributor

snicoll commented Oct 1, 2021

Let's fix this one separately. The problem is that we add a reflection hint for MongoClient (the interface) and, at runtime, we ask for a method based on the actual bean instance type (that implements MongoClient).

GraalVM is quite picky there and we have to ask the method on the interface type. Unfortunately, ReflectionUtils only look for super classes (and not interface) because that's all that's needed on a regular JVM.

I'll fix this by doing an extra check based on the bean definition type as it should give the same behavior as the one used to register the reflection entry.

@snicoll snicoll closed this as completed in 65a5cd1 Oct 1, 2021
@snicoll
Copy link
Contributor

snicoll commented Oct 1, 2021

AutoCloseable is going to be handled in #1124

@snicoll snicoll reopened this Oct 1, 2021
@snicoll snicoll closed this as completed in 455a828 Oct 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug A general bug
Development

No branches or pull requests

3 participants