Hi Simone, Willem,
First of all, timeouts per request were introduced by my patch contributed to
JETTY-912 (Jetty 6). Somebody also ported these changes to Jetty 7. I called the value attached to the exchange 'delay' and not 'timeout'. I wanted these values to be mapped directly to the interface that the Jetty's Timeout class exposed and didn't like the idea of being forced to refactor the Timeout class itself . This HttpExchange's 'delay' was later renamed to 'timeout' while technically it was still a delay. It wasn't my intention to call it that way - to call it 'timeout' .
The approach that you implemented (HttpExchange.timeout overrides HttpClient.timeout) is definitely cleaner. It's more intuitive and covers my usecase. However, I'm concerned with the complexity it introduced to Jetty client's code. Now, when a task is scheduled, HttpClient.timeout is first subtracted from HttpExchange.timeout and later added to the new value inside the Timeout class, right? . I'm also wondering how is a situation when HttpClient.timeout>HttpExchange.timeout handled. I haven't done a full code review, as I don't have Jetty 7 trunk checked out, but I'll need to check how Jetty 7 behaves in such a scenario.
SO timeout & non-blocking channels:
I'm not a NIO guru, but I agree that if a socket channel is running in a non-blocking mode, so_timeout should not be used at all. That's intuitive . Only non-blocking operation (like select()) should be used, so timeout should have no effect.
However, with the old (Jetty 7.2.2) code:
SocketChannel channel = SocketChannel.open();
Address address = destination.isProxied() ? destination.getProxy() : destination.getAddress();
channel.configureBlocking( true );
_selectorManager.register( channel, destination );
we observed weird behavior. It looked as if the client (the select connector) was closing the connection and server was unable to return response. This happened after a really short time as HttpClient.timeout was small and it was used as soTimeout. At the same time, after client changed the state of the socket so that server was unable to return response, HttpClient did not let our client know that something wrong happened. None of te HttpExchange callback methods was called. As channel.socket().setSoTimeout() is no longer used in SelectConnector, it's not really a problem. However, I'd be nice to understand what was going on as there's still a default value for soTimeout. If it's used, it might cause some issues in the future.
Maybe changing the mode from blocking to non-blocking introduces some problems? Just a guess.