HTTPCLIENT-2379 - Handle duplicate release() safely in H2SharingConnPool #663
Conversation
arturobernalg
force-pushed
the
HTTPCLIENT-2379
branch
from
03cab39 to
2099b31
Compare
|
@arturobernalg I am afraid this change-set fixes a side-effect and simply hides the underlying problem. As far as I remember (i am still reviewing my code) there should be no un-tracked pool entries for tracked routes. |
@ok2c the entry is dropped after the first release, and the second call tries to hand it back again, triggering the exception... i see. .. |
arturobernalg
force-pushed
the
HTTPCLIENT-2379
branch
from
5f36b89 to
6e0a18f
Compare
| @@ -288,20 +290,14 @@ PoolEntry<T, C> lease() { | |||
| long release(final PoolEntry<T, C> entry, final boolean reusable) { | |||
| lock.lock(); | |||
| try { | |||
There was a problem hiding this comment.
@arturobernalg Could you please add this check here?
if (!reusable) {
entry.discardConnection(CloseMode.GRACEFUL);
}
7d37f45 to
3f14970
Compare
| .min(Comparator.comparingLong(e -> e.getValue().get())) | ||
| .map(Map.Entry::getKey) | ||
| .map(e -> { |
There was a problem hiding this comment.
@arturobernalg Perhaps this is better?
return entryMap.entrySet().stream()
.filter(e -> {
final C conn = e.getKey().getConnection();
return conn != null && conn.isOpen();
})
.min(Comparator.comparingLong(e -> e.getValue().get()))
.map(e -> {
e.getValue().incrementAndGet();
return e.getKey();
})
.orElse(null);There was a problem hiding this comment.
@yhzdys idd. I just made the change. please check
…t and keeps the entry in the map until the count reaches zero, regardless of whether the connection is still open. The entry is handed back to the parent pool exactly once—on that final release—and any attempt to release an entry that was never leased now raises an IllegalStateException.
arturobernalg
force-pushed
the
HTTPCLIENT-2379
branch
from
10012b1 to
8aed624
Compare
|
@ok2c, it looks like the |
@arturobernalg It does appear that way. CC @rschmitt |
@rschmitt you can find here another example https://github.com/apache/httpcomponents-client/actions/runs/16097757606 |
|
@ok2c cherry pick to |
@arturobernalg Yes, please. |
Done |
…t and keeps the entry in the map until the count reaches zero, regardless of whether the connection is still open. The entry is handed back to the parent pool exactly once—on that final release—and any attempt to release an entry that was never leased now raises an IllegalStateException. (#663) (cherry picked from commit b1efda3)
|
Taking a look, I'll try to get a thread dump |
|
The fix is trivial, I'll have it out in a second |
Return early in H2SharingConnPool#release when the entry’s lease count is zero, so duplicate releases become no-ops and the underlying pool no longer throws
IllegalStateException.tested with https://github.com/yhzdys/httpcomponents-client/commit/c8256b33c3adb45cfa2a49efb2dacfb611da9ac1