New RequestScopeHelper class to track request scopes across threads #2856
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
Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
… of thread local variable in Jersey to make sure correct InjectionManager is used in non-request threads. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
…ultiple applications. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
tjquinno
reviewed
Mar 14, 2021
Comment on lines
+301
to
+302
| requestScopeHelper = new RequestScopeHelper(); | ||
| requestScopeHelper.saveScope(); |
There was a problem hiding this comment.
Would we ever legitimately instantiate a helper without also saving the scope right away?
If not, maybe fold the save work into the constructor? Or make the constructor private and use a static factory method that instantiates the helper and then invokes its saveScope method?
That would not work if there are ever cases where a single helper instance is created and then reused through several save and clear cycles.
There was a problem hiding this comment.
I don't have a use case for saving/clearing/saving yet, so it is a bit more generic than necessary. Having said that, I think it easier to read with an explicit call to saveScope.
There was a problem hiding this comment.
My concern was that correct usage requires me to remember to both instantiate the helper and invoke saveScope explicitly. This is internal so I'm not overly worried.
|
Can you rename this PR to something meaningful and use |
spericas
changed the title
tjquinno
approved these changes
Mar 16, 2021
This was referenced Mar 16, 2021
paulparkinson
pushed a commit
that referenced
this pull request
Mar 29, 2021
…2856) * Run concurrent requests for each test. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * New test. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * New helper class to manage request scope logic across threads. Update of thread local variable in Jersey to make sure correct InjectionManager is used in non-request threads. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * Updated test to show the problem described in #2632 using multiple applications. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * Fixed copyright. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
barchetta
pushed a commit
that referenced
this pull request
Mar 30, 2021
…2856) * Run concurrent requests for each test. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * New test. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * New helper class to manage request scope logic across threads. Update of thread local variable in Jersey to make sure correct InjectionManager is used in non-request threads. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * Updated test to show the problem described in #2632 using multiple applications. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * Fixed copyright. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
aseovic
pushed a commit
to aseovic/helidon
that referenced
this pull request
Apr 26, 2021
…elidon-io#2856) * Run concurrent requests for each test. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * New test. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * New helper class to manage request scope logic across threads. Update of thread local variable in Jersey to make sure correct InjectionManager is used in non-request threads. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * Updated test to show the problem described in helidon-io#2632 using multiple applications. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com> * Fixed copyright. Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
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.
New class in FT that isolates code related to saving and restoring request scopes. The new class
RequestScopeHelperprovides support for CDI, HK2 and multiple JAX-RS applications; it includes additional code to track injection managers across threads. New test that verifies issue reported in #2632. For more information, see #2632 (comment)