ffang commented on code in PR #3645:
URL: https://github.com/apache/cxf/pull/3645#discussion_r4426251591
##########
rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java:
##########
@@ -1614,18 +1616,43 @@ private int getMaxRetransmits() {
* @throws IOException
*/
protected void handleResponse() throws IOException {
- // Process retransmits until we fall out.
- handleRetransmits();
-
- if (outMessage == null
- || outMessage.getExchange() == null
- || outMessage.getExchange().isSynchronous()) {
+ if (isSynchronous()) {
+ // Process retransmits until we fall out.
+ handleRetransmits();
handleResponseInternal();
} else {
+ // the retransmits wait for the response, so they are
processed with it,
+ // not to block the caller thread
handleResponseAsync();
}
}
+ private boolean isSynchronous() {
+ return outMessage == null
+ || outMessage.getExchange() == null
+ || outMessage.getExchange().isSynchronous();
+ }
+
+ /**
+ * Processes the retransmits, if any, and then the response of an
asynchronous exchange,
+ * on the thread that handles the response.
+ */
+ private void handleRetransmitsAndResponse() throws IOException {
Review Comment:
Thanks @reta, good point, having the retransmits in two places with
different behaviour was confusing. I've simplified it so both kinds of exchange
go through the same method, `handleRetransmitsAndResponse()`, and the only
difference left is which thread runs it:
- Synchronous: `close()` → `handleResponse()` →
`handleRetransmitsAndResponse()`, on the caller thread.
- Asynchronous: `close()` → `handleResponse()` → `handleResponseAsync()` →
`handleResponseOnWorkqueue()` → `handleRetransmitsAndResponse()`, on the thread
that handles the response.
`handleRetransmits()` is now called only from
`handleRetransmitsAndResponse()`. That method also always releases the cached
request, right after the retransmits, so `close()` no longer needs to know
which path ran. It only releases the cached request as a fallback if
`handleResponse()` throws, for example when the response can't be handed to the
executor.
For synchronous exchanges, the cached request is now released after the
retransmits rather than after the whole response has been handled.
`handleResponseInternal()` already releases it before reading a normal
response, so nothing uses it after the retransmits.
It's in https://github.com/apache/cxf/pull/3645/commits/ced29ea56b. The full
`systests/transports` suite passes in both the default and `forceURLConnection`
runs, including `AsyncRetransmitTest` and `testAutoRedirect[hc]` /
`testAuthenticationChallenge[hc]`. So do the `hc5` `AsyncRetransmitTest` and
the unit tests of the `http`, `http-hc` and `http-hc5` modules.
Best Regards
Freeman
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
