Skip to content

Fix. retry is incremented by twice #1150

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

Conversation

charliechang
Copy link

@charliechang charliechang commented May 3, 2016

There is a code defect in counting retry.

There are two places which can increment the counter.
NettyResponseFuture.java

    public boolean canRetry() {
        return maxRetry > 0 && CURRENT_RETRY_UPDATER.incrementAndGet(this) <= maxRetry;
    }
    public boolean canBeReplayed() {
        return !isDone() && canRetry() && !(Channels.isChannelValid(channel) && !getUri().getScheme().equalsIgnoreCase("https")) && !inAuth.get() && !inProxyAuth.get();
    }

NettyConnectListener.java

        boolean canRetry = future.canRetry(); //Add retry counter here
        LOGGER.debug("Trying to recover from failing to connect channel {} with a retry value of {} ", channel, canRetry);
        if (canRetry//
                && cause != null 
                && (future.getChannelState() != ChannelState.NEW || StackTraceInspector.recoverOnNettyDisconnectException(cause))) {

            if (requestSender.retry(future)) { //Add retry counter here either, since canBeReplayed will be called
                return;
            }
        }

The retry counter is added in future.canRetry() and requestSender.retry(future)
This PR fixed the problem and rename the method from canRetry to incRetryAndCheck for better naming convention

@slandelle
Copy link
Contributor

Thanks!

@slandelle slandelle merged commit a57bf47 into AsyncHttpClient:master May 3, 2016
slandelle added a commit that referenced this pull request May 3, 2016
@slandelle slandelle added this to the 2.0.3 milestone May 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants