You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Michael Becke (JIRA)" <ji...@apache.org> on 2007/02/14 23:55:05 UTC
[jira] Commented: (HTTPCLIENT-633)
MultiThreadedHttpConnectionManager does not properly respond to thread
interrupts
[ https://issues.apache.org/jira/browse/HTTPCLIENT-633?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473237 ]
Michael Becke commented on HTTPCLIENT-633:
------------------------------------------
Hi John,
I would agree, using object notification is probably a cleaner way to do this. I'm not sure why I did it this way originally, simplicity perhaps. My question is if interrupting a waiting thread outside of the connection manager is something we want to support. If we did, would we just throw a ConnectionPoolTimeoutException when it occurs? This would seem to break the contract of doGetConnection. I'm not sure how we will handle HttpConnection getConnection(HostConfiguration hostConfiguration) without an API change.
Assuming we did want to change this, we would need to wait on the HostConnectionPool instead of the ConnectionPool. Doing this brings up a couple of potential issues, but I think they're all fixable. In particular:
- We would have two layers of synchronization to go through, ConnectionPool and HostConnectionPool, and would need to handle this carefully to avoid potential dead-locks, race conditions, etc.
- We lose the FIFO nature currently guaranteed to waiting threads. Object.notify() makes no guarantees about which thread will be woken first.
To get this into 3.1 we will definitely want to have an RC release with this change first. Given the complexity of synchronization I would want to give it some time for testing.
Any other thoughts?
Mike
> MultiThreadedHttpConnectionManager does not properly respond to thread interrupts
> ---------------------------------------------------------------------------------
>
> Key: HTTPCLIENT-633
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-633
> Project: HttpComponents HttpClient
> Issue Type: Bug
> Components: HttpConn
> Affects Versions: 3.1 Beta 1
> Reporter: John Goodwin
>
> MultiThreadedHttpConnectionManager uses interrupts to notify waiting threads when a connection is ready for them. Issues arise if the threads are interrupted by someone else while they are still waiting on a thread, because doGetConnection does not remove the threads from the queue of waiting threads when they are interrupted:
> connectionPool.wait(timeToWait);
> // we have not been interrupted so we need to remove ourselves from the
> // wait queue
> hostPool.waitingThreads.remove(waitingThread); connectionPool.waitingThreads.remove(waitingThread);
> } catch (InterruptedException e) {
> // do nothing } finally {
> if (useTimeout) {
> endWait = System.currentTimeMillis();
> timeToWait -= (endWait - startWait); } }
> Under ordinary circumstances, the queue maintenance is done by the notifyWaitingThread method. However, if the thread is interrupted by any other part of the system, it will (1) not actually be released, since the loop in doGetConnection will force it back to the wait, and (2) will be added the waiting thread to the queue repeatedly, which basically means that the thread will eventually receive the interrupt from notifyWaitingThread at some later point, when it is no longer actually waiting for a connection.
> This code could probably be re-architected to make it less error-prone, but the fundamental issue seems to be the use of interrupts to signal waiting threads, as opposed to something like a notify.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: [jira] Commented: (HTTPCLIENT-633) MultiThreadedHttpConnectionManager does not properly respond to thread interrupts
Posted by Michael Becke <mb...@gmail.com>.
Hi Roland,
> We can't prevent it, so we should at least make sure that it doesn't screw up
> our internal data structures. Whether we'll keep the thread waiting in a loop
> or whether we'll cancel waiting is up for debate. I prefer the cancelling.
I agree. The key issue is making sure interrupts don't break the
connection manager. I did some more digging about our options and
will post my thoughts on JIRA in a few minutes.
> ConnectionPoolInterruptedException extends ConnectionPoolTimeoutException?
> Sun has an InterruptedIOException for exception mapping purposes.
That might be a good option.
> The API change will not affect users in cases where the current code works
> correctly. In cases where the current code runs into a bug, I guess throwing
> an exception is the lesser evil.
My preference would be to not break the API for something relatively
uncommon like this. In my opinion, our choices would be to throw a
runtime exception or just continue to wait.
> I'd rather stick with the interrupt technique than switch to wait/notify
> in 3.1. The code will undergo a major revamp for 4.0 anyway, that is the
> occasion to change such fundamental behavior. The current implementation
> has worked well so far.
I agree moving away from interrupt brings up a whole host of potential
issues. I will detail this in JIRA.
Thanks,
Mike
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: [jira] Commented: (HTTPCLIENT-633) MultiThreadedHttpConnectionManager
does not properly respond to thread interrupts
Posted by Roland Weber <ht...@dubioso.net>.
Hi Mike,
> My question is if interrupting a waiting thread outside of the connection manager
> is something we want to support.
We can't prevent it, so we should at least make sure that it doesn't screw up
our internal data structures. Whether we'll keep the thread waiting in a loop
or whether we'll cancel waiting is up for debate. I prefer the cancelling.
> If we did, would we just throw a ConnectionPoolTimeoutException when it occurs?
ConnectionPoolInterruptedException extends ConnectionPoolTimeoutException?
Sun has an InterruptedIOException for exception mapping purposes.
> This would seem to break the contract of doGetConnection. I'm not sure how we
> will handle HttpConnection getConnection(HostConfiguration hostConfiguration)
> without an API change.
The API change will not affect users in cases where the current code works
correctly. In cases where the current code runs into a bug, I guess throwing
an exception is the lesser evil.
> Assuming we did want to change this, we would need to
> [snipped some scary things]
I'd rather stick with the interrupt technique than switch to wait/notify
in 3.1. The code will undergo a major revamp for 4.0 anyway, that is the
occasion to change such fundamental behavior. The current implementation
has worked well so far.
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org