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

Continuations (requests) are not discarded on timeouts #375

Closed
wheezil opened this issue Jun 27, 2018 · 0 comments
Closed

Continuations (requests) are not discarded on timeouts #375

wheezil opened this issue Jun 27, 2018 · 0 comments
Assignees
Milestone

Comments

@wheezil
Copy link

wheezil commented Jun 27, 2018

I am starting to use the RpcClient class, and examining the source it apparently can leak by adding entries to _continuationMap that are never removed if there is no reply. If doCall() throws TimeoutException, there is no reason to leave this entry in the map as it will never be used, so it should catch TimeoutException and remove the entry.

  • RabbitMQ version: 5.3.0 (but issue remains in master code)

Relevant code follows. The call to k.uninterruptibleGet() can throw TimeoutException. We can catch that and remove the entry before re-throwing it.

public Response doCall(AMQP.BasicProperties props, byte[] message, int timeout)
    throws IOException, ShutdownSignalException, TimeoutException {
    checkConsumer();
    BlockingCell<Object> k = new BlockingCell<Object>();
    synchronized (_continuationMap) {
        _correlationId++;
        String replyId = "" + _correlationId;
        props = ((props==null) ? new AMQP.BasicProperties.Builder() : props.builder())
            .correlationId(replyId).replyTo(_replyTo).build();
        _continuationMap.put(replyId, k);
    }
    publish(props, message);
    Object reply = k.uninterruptibleGet(timeout);
    if (reply instanceof ShutdownSignalException) {
        ShutdownSignalException sig = (ShutdownSignalException) reply;
        ShutdownSignalException wrapper =
            new ShutdownSignalException(sig.isHardError(),
                                        sig.isInitiatedByApplication(),
                                        sig.getReason(),
                                        sig.getReference());
        wrapper.initCause(sig);
        throw wrapper;
    } else {
        return (Response) reply;
    }
}

I've already posted this to the rabbitmq mailing list, and got the thumbs-up to submit a patch, so here goes! Please forgive my newbie status; I am still struggling getting this to build.

@michaelklishin michaelklishin changed the title RpcClient has potential leak in doCall() : _continuationMap can grow without bounds Continuations (requests) are not discarded on timeouts Jun 27, 2018
@acogoluegnes acogoluegnes added this to the 5.3.1 milestone Jun 28, 2018
acogoluegnes added a commit that referenced this issue Jun 28, 2018
Add a test as well to check the outstanding request map is cleaned after
a timeout.

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

No branches or pull requests

3 participants