Thread-safe ServiceLoader usage #892
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… post-0.11.0 behavior using the JDK ServiceLoader to find/create instances of an SPI interface. This change: - Reinstates the <= 0.10.x behavior of caching application singleton service implementation instances in a thread-safe reference (previously an AtomicReference, but in this change, a ConcurrentMap). If an app singleton instance is cached and found, it is returned to be (re)used immediately when requested. This is ok for JJWT's purposes because all service implementations instances must be thread-safe application singletons by API contract/design, so caching them for repeated use is fine. - Ensures that only if a service implementation instance is not in the app singleton cache, a new instance is located/created using a new JDK ServiceLoader instance, which doesn't require thread-safe considerations since it is used only in a single-threaded model for the short time it is used to discover a service implementation. This PR/change removes the post-0.11.0 concurrent cache of ServiceLoader instances since they themselves are not designed to be thread-safe. - Ensures that if a ServiceLoader discovers an implementation and returns a new instance, that instance is then cached as an application singleton in the aforementioned ConcurrentMap for continued reuse. - Renames Services#loadFirst to Services#get to more accurately reflect calling expectations: The fact that any 'loading' via the ServiceLoader may occur is not important for Services callers, and the previous method name was unnecessarily exposing internal implementation concepts. This is safe to do in a point release (0.12.3 -> 0.12.4) because the Services class and its methods, while public, are in the `impl` module, only to be used internally for JJWT's purpose and never intended to be used by application developers. - Updates all test methods to use the renamed method accordingly.
bdemers
reviewed
Jan 17, 2024
bdemers
approved these changes
Jan 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Blend of pre-0.11.0 behavior that cached implementation instances and post-0.11.0 behavior using the JDK ServiceLoader to find/create instances of an SPI interface. This change:
Reinstates the <= 0.10.x behavior of caching application singleton service implementation instances in a thread-safe reference (previously an
AtomicReference, but in this change, aConcurrentMap). If an app singleton instance is cached and found, it is returned to be (re)used immediately when requested. This is ok for JJWT's purposes because all service implementations instances must be thread-safe application singletons by API contract/design, so caching them for repeated use is fine.Ensures that only if a service implementation instance is not in the app singleton cache, a new instance is located/created using a new JDK
ServiceLoaderinstance, which doesn't require thread-safe considerations since it is used only in a single-threaded model for the short time it is used to discover a service implementation. This PR/change removes the post-0.11.0 concurrent cache ofServiceLoaderinstances since they themselves are not designed to be thread-safe.Ensures that if a
ServiceLoaderdiscovers an implementation and returns a new instance, that instance is then cached as an application singleton in the aforementionedConcurrentMapfor continued reuse.Renames
Services#loadFirsttoServices#getto more accurately reflect calling expectations: The fact that any 'loading' via aServiceLoadermay occur is not important forServicescallers, and the previous method name was unnecessarily exposing internal implementation concepts. This is safe to do in a point release (0.12.3 -> 0.12.4) because theServicesclass and its methods, while public, are in theimplmodule, only to be used internally for JJWT's purpose and never intended to be used by application developers.Updates all test methods to use the renamed method accordingly.
Fixes #873