Github user dmap commented on the issue:
https://github.com/apache/httpcomponents-core/pull/94
@ok2c I'm happy with that. Thanks for the help.
The changes in `ClientHttpProtocolNegotiator` were required because it
obviously needs to write content (when it needs to write the preface) but never
set the OP_WRITE flag on the session. This used to work 'by accident' because
of the way that an enqueued command automatically sets the OP_WRITE flag. But
when the SOCKS protocol handler runs it also sets and clears this flag several
times, and when finished leaves it cleared (because the last thing it does is
read the SOCKS connect repsonse). So without explicitly setting the flag in
the `ClientHttpProtocolNegotiator` it could hang forever waiting to write the
preface. I reproduced this in some of the unit tests under Java 11.
This is an example of what I referred to as the `awkward` handling of the
interest ops. I believe that in general only the IOEventHandlers should set
the interest ops because they are the only actual consumers of the IO events.
In particular I don't like the way the enqueue command sets the OP_WRITE flag,
it should instead fire a different event that an interested IOEventHandler can
listen for and then the IOEventHandler can set OP_WRITE if it wishes.
Because of this awkward management of the interest ops an IOEventHandler
can receive an inputReady() or (in particular) an outputReady() callback when
it is not expecting it. I had to add additional code to my SOCKS handler to
avoid an infinite loop when I got an outputReady() (because of the
enqueueCommand done by another thread) when the SOCKS handler was actually
wanting input, not output.
Anyway, that is all probably best done as a different change. It wouldn't
be hard to clean up, but it isn't strictly speaking required for this PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
