[
https://issues.apache.org/jira/browse/HTTPCLIENT-2033?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16989380#comment-16989380
]
Aapo Laitinen commented on HTTPCLIENT-2033:
-------------------------------------------
{quote}I think the {{isShutdown}} atomic variable should safe-guard against
such race condition.
{quote}
[~olegk] Checking {{isShutDown}} in {{requestConnection(...)}} helps, but I
couldn't find anything preventing this interleaving of events:
# {{isShutdown}} is found to be false in {{requestConnection(...)}}
# {{isShutdown}} is set to true in {{shutdown()}}
# {{this.pool.enumLeased(...)}} is invoked in {{shutdown()}} to shutdown all
connections.
# {{this.pool.lease(...)}} is invoked in {{requestConnection(...)}}
# {{this.pool.shutdown()}} is invoked in {{shutdown()}}
Though now that I think of it, I failed to appreciate that
{{PoolingHttpClientConnectionManager}} owns the pool. One possibility here
would be to move the {{isShutdown}} check in {{requestConnection(...)}} below
the {{this.pool.lease(...)}} call and immediately cancel the future if the
check fails. In other words, either we leased the connection before the shut
down was initiated, in which case it will be shut down when the connections are
enumerated, or we managed to lease it after the enumeration in which case we
ensure we won't actually do anything with it.
Of course, for anything ill to come out of this, somebody would also have to
buffer some data with the connection between (4) and (5). It would be highly
unlikely that they would have the time to do so, but as far as I can tell, it's
technically possible.
--Aapo
> Connection manager shutdown flushes output buffer without synchronization
> -------------------------------------------------------------------------
>
> Key: HTTPCLIENT-2033
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-2033
> Project: HttpComponents HttpClient
> Issue Type: Bug
> Components: HttpClient (classic)
> Affects Versions: 4.5.10
> Environment: Linux
> Reporter: Aapo Laitinen
> Priority: Major
> Fix For: 4.5.11
>
>
> We've observed cases of garbled data sent over the wire that seem to
> correlate with calling {{HttpClientConnectionManager#shutdown()}} while
> requests are in progress. For example, a PUT request with entity {{ABCD}}
> (each letter corresponding to 4 kilobytes of data) may be received as
> {{ABC{color:#172b4d}B{color}}} (the last block replaced with the second
> block).
> Omitting some intermediate layers, {{HttpClientConnectionManager#shutdown()}}
> proceeds to call {{AbstractConnPool#shutdown()}},
> {{BHttpConnectionBase#close()}}, {{SessionOutputBufferImpl#flush()}} and
> finally {{SessionOutputBufferImpl#flushBuffer()}}. As far as I can tell,
> there is nothing to stop the thread that is performing the shutdown from
> invoking {{flushBuffer()}} while another thread is writing data to the buffer
> and/or flushing it.
> *How to reproduce*
> I haven't been able to reliably reproduce the garbled data, but I confirmed
> the lack of synchronization as follows:
> 1. Modified {{flushBuffer()}} so that it will try to a acquire and release a
> shared lock before proceeding and will report any cases of two threads
> attempting to flush the same buffer at the same time.
> 2. Acquired the lock while another thread was making requests, making the
> other thread block inside {{flushBuffer()}}.
> 3. Called {{HttpClientConnectionManager#shutdown()}}
> 4. Observed that the thread doing the shutdown entered {{flushBuffer()}}
> while the thread that was making requests hadn't exited it yet.
> *What I was expecting to happen*
> I'm not sure what the expected behavior of calling
> {{HttpClientConnectionManager#shutdown()}} is, but either of flushing the
> buffer with proper synchronization or abruptly closing the connection
> ({{BHttpConnectionBase#shutdown()}} instead of {{close()}}) strike me as
> acceptable. Basically, I'd expect that the received data might be truncated
> (e.g. {{AB}} or {{ABC}} instead of {{ABCD}}) but never garbled (e.g. {{ABCB}}
> or {{ABCC}} instead of {{ABCD}}).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
