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

Migration of CDI contextual beans into new threads #3332

Merged
merged 4 commits into from
Sep 1, 2021

Conversation

spericas
Copy link
Member

@spericas spericas commented Aug 27, 2021

Additional code is provided to not only active the request scope in the new thread but also migrate the beans in request scope. This is done according to the Weld documentation on context migration.

A new test shows how a request-scoped bean created in thread A is the same bean available in thread B using FT's @Asynchronous annotation. Note that the request-scoped bean in thread A needs to be accessed for its proxy to be created and migration into thread B to be successful.

Bean migration is one directional: from thread A (original request thread) to thread B and not the other way around. Application developers are responsible for properly synchronizing access to these beans that can potentially be accessed from multiple threads.

Issue #3319

…ntextual instance in reuqest scope and migrates them to the new thread. Before we were activating the scope but without migrating the instances.

Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
…FT. Uses @asynchronous to verify beans are the same (share same secret) in request scope.
@spericas spericas added this to the 2.4.0 milestone Aug 27, 2021
@spericas spericas self-assigned this Aug 27, 2021
Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
@ljnelson
Copy link
Member

Haven't even looked at this but I know that Weld does context propagation; you may want to double-check to see if one of its methods can help here so future bugs end up living where they're supposed to. (These methods may not be available in the version of Weld that Helidon uses; not sure. I just didn't want to lose the thread, so to speak.)

@spericas
Copy link
Member Author

@ljnelson That's exactly where I got my Inspiration, that link. I've adapted their code for the migration process, excluding the other scopes they show there. Interestingly, Jersey has similar code in WeldRequestScope, however, they don't actually call clearAndSet ever, which sees very strange. For that reason, I've decided to do this by hand in Helidon instead of leveraging that Jersey class.

Could you think of a better way to do this migration than what is shown in this PR?

@spericas spericas requested a review from ljnelson August 31, 2021 13:45
@spericas spericas removed the wip label Aug 31, 2021
@spericas spericas changed the title WIP: Migration of CDI contextual beans into new threads Migration of CDI contextual beans into new threads Aug 31, 2021
Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
@tomas-langer
Copy link
Member

@spericas is this something specific to Fault tolerance? Could we use the same code to have a general way of enabling request context in any asynchronous code when using CDI?

Copy link
Member

@tjquinno tjquinno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go!

@spericas spericas merged commit c0f17a3 into helidon-io:master Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants