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

ServletContext atomic register of bean #22929

Closed
wants to merge 1 commit into from
Closed

ServletContext atomic register of bean #22929

wants to merge 1 commit into from

Conversation

lmagyar89
Copy link

As far as I can see the registering beans to javax.servlet.ServletContext is not atomic.
Destruction callback registration has some thread-safe issues as it using not thread safe collection.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 8, 2019
@jhoeller jhoeller self-assigned this May 8, 2019
@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 9, 2019
@jhoeller jhoeller added this to the 5.2 M3 milestone May 9, 2019
Atomic bean creation on ServletContextScope
Thread-safe destructionCallbacks operations
@jhoeller jhoeller modified the milestones: 5.2 M3, 5.2 RC1 Jun 11, 2019
@jhoeller
Copy link
Contributor

On review, our web scope implementations generally don't provide atomic bean instance guarantees, neither for request nor for session scopes and therefore arguably not for the ServletContext scope either. Even the destroy callbacks are just a scope cleanup mechanism, with the then-latest bean instance to be processed but no individual destruction guarantees for beans that got programmatically removed inbetween. If you need atomic bean instance handling, your primary choice is the singleton scope.

That said, you got a point that the destruction callback handling in ServletContextScope has a thread-safe visibility issue and needs some level of synchronization, at least if it is meant to be used for lazy bean initialization at runtime (whereas it was originally only really intended for web application startup). Even beyond that, we should consistently remove the destruction callbacks before we remove the instance attributes themselves, since otherwise we may - in a race condition - remove a newly registered destruction callback from some other thread that triggered re-creation of the same scoped bean. I'll deal with this in a separate GitHub issue, to be consistently handled across all web scope implementations.

@jhoeller
Copy link
Contributor

See #23117 for consistent removal of destruction callbacks, also including basic synchronization in ServletContextScope (but just for thread-safe visibility, with no atomic bean instance guarantees).

Thanks for the pull request, in any case!

@jhoeller jhoeller closed this Jun 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants