You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by "dennis-yemelyanov (via GitHub)" <gi...@apache.org> on 2024/04/01 23:12:21 UTC
[PR] reuse the same WebClient instance and fix CXF-8992 if it's threadsafe [cxf]
dennis-yemelyanov opened a new pull request, #1781:
URL: https://github.com/apache/cxf/pull/1781
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reuse the same WebClient instance and fix CXF-8992 if it's threadsafe [cxf]
Posted by "dennis-yemelyanov (via GitHub)" <gi...@apache.org>.
dennis-yemelyanov commented on code in PR #1781:
URL: https://github.com/apache/cxf/pull/1781#discussion_r1554900848
##########
rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/spec/ClientImpl.java:
##########
@@ -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);
Review Comment:
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 https://github.com/apache/cxf/commit/5ffdb906b560458ad8df5a892554b6e806d98d9e#diff-660ab7b7b6a7b75d899f2a1cd889b168aadd5a194d2aa9fbae875c1b56891c6c
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reuse the same WebClient instance and fix CXF-8992 if it's threadsafe [cxf]
Posted by "paulbors (via GitHub)" <gi...@apache.org>.
paulbors commented on code in PR #1781:
URL: https://github.com/apache/cxf/pull/1781#discussion_r1954938484
##########
rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/spec/ClientImpl.java:
##########
@@ -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);
Review Comment:
Or..... you can make a deep copy of the WebClient inside of WebClient.fromClient() instead of a shallow copy and you never have to worry about any of this (since its an immutable copy of it)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reuse the same WebClient instance and fix CXF-8992 if it's threadsafe [cxf]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet closed pull request #1781: reuse the same WebClient instance and fix CXF-8992 if it's threadsafe
URL: https://github.com/apache/cxf/pull/1781
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reuse the same WebClient instance and fix CXF-8992 if it's threadsafe [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #1781:
URL: https://github.com/apache/cxf/pull/1781#discussion_r1550639711
##########
rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/spec/ClientImpl.java:
##########
@@ -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);
Review Comment:
@dennis-yemelyanov the usage of the new client instance is not accidental and is not about thread safety as much as the following the specification JAX-RS (please consult section "5.3 Client Targets" of the JSR-339).
##########
rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/spec/ClientImpl.java:
##########
@@ -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);
Review Comment:
@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).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reuse the same WebClient instance and fix CXF-8992 if it's threadsafe [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #1781:
URL: https://github.com/apache/cxf/pull/1781#discussion_r1550639711
##########
rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/spec/ClientImpl.java:
##########
@@ -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);
Review Comment:
@dennis-yemelyanov the usage of the new client instance is not accidental and is not about thread safety as much as the following the specification JAX-RS spec (please consult section "5.3 Client Targets" of the JSR-339).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] reuse the same WebClient instance and fix CXF-8992 if it's threadsafe [cxf]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1781:
URL: https://github.com/apache/cxf/pull/1781#issuecomment-4033218866
Closing — CXF-8992 was already fixed via PR #2281 (merged March 2025) using a reference-counting approach. This PR's approach also violates JAX-RS spec requirements for WebTarget immutability, as acknowledged by the author.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org