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

reuse the same WebClient instance and fix CXF-8992 if it's threadsafe #1781

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dennis-yemelyanov
Copy link

When the WebClient is thread safe, it should be fine to reuse it instead of calling WebClient.fromClient(). Additionally, this fixes (at least in cases when the WebClient is thread safe) an issue due to GC: https://issues.apache.org/jira/browse/CXF-8992

When the WebClient is thread safe, it should be fine to reuse it instead of calling WebClient.fromClient(). Additionally, this fixes (at least in cases when the WebClient is thread safe) an issue due to GC: https://issues.apache.org/jira/browse/CXF-8992
@@ -470,7 +472,7 @@ public WebTarget resolveTemplatesFromEncoded(Map<String, Object> templatesMap) {
private WebTarget newWebTarget(UriBuilder newBuilder) {
WebClient newClient;
if (targetClient != null) {
newClient = WebClient.fromClient(targetClient);
newClient = threadSafeTargetClient ? targetClient : WebClient.fromClient(targetClient);
Copy link
Member

@reta reta Apr 3, 2024

Choose a reason for hiding this comment

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

@dennis-yemelyanov the usage of the new client instance is not accidental and is not about thread safety as much as the following the JAX-RS specification (please consult section "5.3 Client Targets" of the JSR-339).

Copy link
Author

Choose a reason for hiding this comment

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

ah WebTarget must be immutable, I missed that part... I might think more if it's possible to separate the externally visible part of it (like the target URL) from internal details, so the closeable (and finalizable) part can still be reused. But I guess it won't be trivial (if possible at all).

Or we need to adjust some logic from this commit, to ensure different instances don't affect each other when finalized 5ffdb90#diff-660ab7b7b6a7b75d899f2a1cd889b168aadd5a194d2aa9fbae875c1b56891c6c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants