[GenAI] Fixing 11656: Handling the issue with @PostConstruct annotated bean #12159
Conversation
Jarvx
changed the title
Jarvx
changed the title
| // If this instance was created by this context earlier, it may already have been injected/initialized. | ||
| // In such case skip duplicate injection/initialization. Use the beansCreatedByContext set to detect this. | ||
| boolean createdByThisContext; | ||
| synchronized (beansCreatedByContext) { |
There was a problem hiding this comment.
why you use synchronizedSet together with synchronized block?
There was a problem hiding this comment.
Oh. Collections.synchronizedSet(...) already ensures that all accesses
so the block is redundant. I’ll remove the extra synchronization. Thanks.
|
It shouldn't matter if the bean is created by the bean context or not, the triggering of the post construct should act the same if the bean is create manually or not. Not sure for the current behaviour. I would expect that |
| public class DoublePostConstructTest { | ||
|
|
||
| @Test | ||
| void postConstructCalledOnceWhenCreateAndRegisterSingleton() { |
There was a problem hiding this comment.
This PR may result in a memory leak since each call to createBean will create a new instance to be tracked forever. This is not the current behaviour of createBean which relies on the user to invoke post construct hooks.
It is actually debatable whether this is even a bug
| // If this instance was created by this context earlier, it may already have been injected/initialized. | ||
| // In such case skip duplicate injection/initialization. Use the beansCreatedByContext set to detect this. | ||
| boolean createdByThisContext; | ||
| synchronized (beansCreatedByContext) { |
There was a problem hiding this comment.
introduction of synchronisation on the createBean path will likely result in performance degradation
|
I have updated this PR by adding new fixes and new reproducer test file. |
|
As I wrote before, tests need to be in Java for the core functionality |
4 participants
Issue link: #11656
Issue description
The test reproduces the issue: it creates a bean via
ApplicationContext.createBean(which should invoke@PostConstruct once), then registers the same instance via registerSingletonand asserts that@PostConstructis not invoked a second time. On the original code, the test failed with initCount=2, confirming the double @PostConstruct invocation.The patch resolves the issue by tracking instances created by the context (via
createBean) in an identity-based set. When registerSingleton is called withinject=true, it checks this set: if the instance was already initialized by the context, it performs injection only (without initialization), avoiding a second@PostConstructcall. The test passes after the patch, and the full test suite builds successfully.Note: this approach introduces a global set of initialized instances, which may have implications (e.g., strong references and non-concurrent collection);
Solution
please run the command