On Fri, 11 Mar 2022 10:33:09 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> I'm not completely sure of the purpose of the synchronized block here - but >> I'd rather not change this in this PR (and I'd rather not change it at all >> unless it proves to cause issues). As for the second check for isOpen() - I >> included it here because 1. the check is cheap, and 2. since there is a >> synchronized block - some time may have elapsed waiting for the lock - so >> rechecking here before putting the connection into the pool would save >> adding a dead connection to the pool. > > Sounds fine to me. One last question - I suspect you didn't include the > additional `c.finalStream()` within this synchronized block because that call > is relatively expensive (with `finalStream()` being `synchronized`)? Well - no - I'm not expecting finalStream() to be changed asynchronously while adding the connection to the pool. setFinalStream() is called at very few places, and a connection is offered to the pool only once. On the other hand - the underlying TCP connection could be closed any time by the remote peer. ------------- PR: https://git.openjdk.java.net/jdk/pull/7776
