Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove a unnecessary else in DefaultSingletonBeanRegistry #33502

Closed
wants to merge 3 commits into from
Closed

Remove a unnecessary else in DefaultSingletonBeanRegistry #33502

wants to merge 3 commits into from

Conversation

liuao1004
Copy link

The method getSingleton(String, ObjectFactory<?>) may throw a BeanCurrentlyInCreationException in multiple threads.

If Thread-A acquires the singletonLock, but before it assigns itself to singletonCreationThread,
Thread-B gets a null value at singletonCreationThread, then Thread-A and Thread-B will both invoke beforeSingletonCreation.

In this commit, I remove the else,
so the thread which does not acquire singletonLock will be blocked at the code this.singletonLock.lock().

Closes gh-33463

The method `getSingleton(String, ObjectFactory<?>)` may throw a
`BeanCurrentlyInCreationException` in multiple threads.

If Thread-A acquires the `singletonLock`, but before it assigns itself to
`singletonCreationThread`,
Thread-B gets a null value at `singletonCreationThread`,
then Thread-A and Thread-B will both invoke `beforeSingletonCreation`.

In this commit, I remove the else,
so the thread which does not acquire `singletonLock` will be blocked
at the code `this.singletonLock.lock()`.

Closes gh-33463
@pivotal-cla
Copy link

@liuao1004 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 6, 2024
@pivotal-cla
Copy link

@liuao1004 Thank you for signing the Contributor License Agreement!

@bclozel bclozel added the in: core Issues in core modules (aop, beans, core, context, expression) label Sep 7, 2024
liuao1004 and others added 2 commits September 9, 2024 10:31
The method `getSingleton(String, ObjectFactory<?>)` may throw a
`BeanCurrentlyInCreationException` in multiple threads.

If Thread-A acquires the `singletonLock`, and it assigns itself to
`singletonCreationThread` immediately,
Thread-B will get a not null value at `singletonCreationThread`,
then Thread-A and Thread-B will both invoke `beforeSingletonCreation`.

In this commit, I remove the else,
so the thread which does not acquire `singletonLock` will be blocked
at the code `this.singletonLock.lock()`.

Closes gh-33463
@jhoeller jhoeller self-assigned this Sep 9, 2024
@jhoeller
Copy link
Contributor

That else block is necessary for the lenient locking as covered by BeanFactoryLockingTests. Instead, I am going with a larger revision that resolves #33463 through applying the lenient locking fallback to the singleton pre-instantiation phase during a coordinated bootstrap only, exposing concurrent scenarios after bootstrap (e.g. for lazy singletons) to a full singleton lock.

@jhoeller jhoeller closed this Sep 11, 2024
@jhoeller jhoeller added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
6 participants