NettyHttpClient: Fix double-return on certain exceptions. (#12626)

The "exceptionCaught" handler may get called multiple times. We should
only return the channel to the pool the first time. Returning it more
than once leads to a warning like "Resource at key[%s] was returned
multiple times?"
This commit is contained in:
Gian Merlino 2022-06-14 21:40:47 -07:00 committed by GitHub
parent 1f6e888472
commit 283249c51b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 15 additions and 2 deletions

View File

@ -317,6 +317,8 @@ public class NettyHttpClient extends AbstractHttpClient
}
}
// Ignore return value of setException, since exceptionCaught can be called multiple times and we
// only want to report the first one.
if (event.getCause() instanceof ReadTimeoutException) {
// ReadTimeoutException thrown by ReadTimeoutHandler is a singleton with a misleading stack trace.
// No point including it: instead, we replace it with a fresh exception.
@ -338,7 +340,11 @@ public class NettyHttpClient extends AbstractHttpClient
log.warn(e, "Error while closing channel");
}
finally {
channelResourceContainer.returnResource();
if (channelResourceContainer.isPresent()) {
// exceptionCaught can be called multiple times: we only want to return the channel if it hasn't
// already been returned.
channelResourceContainer.returnResource();
}
}
}

View File

@ -24,5 +24,6 @@ package org.apache.druid.java.util.http.client.pool;
public interface ResourceContainer<ResourceType>
{
ResourceType get();
boolean isPresent();
void returnResource();
}

View File

@ -119,11 +119,17 @@ public class ResourcePool<K, V> implements Closeable
return value;
}
@Override
public boolean isPresent()
{
return !returned.get();
}
@Override
public void returnResource()
{
if (returned.getAndSet(true)) {
log.warn(StringUtils.format("Resource at key[%s] was returned multiple times?", key));
log.warn("Resource at key[%s] was returned multiple times?", key);
} else {
holder.giveBack(value);
}