+1, doesnt make sense to do both to me, in particular with our pooling abstraction
Le 29 août 2016 21:42, "Jonathan Gallimore" <[email protected]> a écrit : > Indeed it is, and I think your change to do that in connection.close() is > actually a better fix (thanks!), but we still have an issue, and that is > the order in Client.java: > > if (null != in) { > try { > in.close(); > } catch (final Throwable e) { > //Ignore > } > } > > if (null != conn) { > try { > conn.close(); > } catch (final Throwable t) { > logger.log(Level.WARNING, "Error closing connection > with server: " + t.getMessage(), t); > } > } > > The inputstream will be closed before conn.close() - effectively closing > the stream while under certain circumstances there is still more to read. > This specifically happens when the response from the server uses > Transfer-encoding: chunked, and the client is connecting over https. The > effect in this case > is that HttpsUrlConnection won't reuse the connection (effectively there is > no keep-alive) and the SSL handshake has to happen all over again on the > next call. > > As I mentioned, I prefer your solution - feels neater. The question is, how > to solve this small ordering issue - how about moving the in.close() / > out.close() logic > to each of the implementations of Connection? (HttpConnection, > SocketConnection and an anonymous class in MockConnectionFactory) > > Thoughts? > > Jon > > > On Mon, Aug 29, 2016 at 10:23 AM, <[email protected]> wrote: > > > Repository: tomee > > Updated Branches: > > refs/heads/master a26d99040 -> 8514d47c5 > > > > > > useless code since done in the connection close > > > > > > Project: http://git-wip-us.apache.org/repos/asf/tomee/repo > > Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/8514d47c > > Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/8514d47c > > Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/8514d47c > > > > Branch: refs/heads/master > > Commit: 8514d47c58b60e8288ccffa28a1bc3f33b4cbdfd > > Parents: a26d990 > > Author: Romain manni-Bucau <[email protected]> > > Authored: Mon Aug 29 11:22:51 2016 +0200 > > Committer: Romain manni-Bucau <[email protected]> > > Committed: Mon Aug 29 11:22:51 2016 +0200 > > > > ---------------------------------------------------------------------- > > .../java/org/apache/openejb/client/Client.java | 17 > ----------------- > > 1 file changed, 17 deletions(-) > > ---------------------------------------------------------------------- > > > > > > http://git-wip-us.apache.org/repos/asf/tomee/blob/8514d47c/ > > server/openejb-client/src/main/java/org/apache/openejb/ > client/Client.java > > ---------------------------------------------------------------------- > > diff --git a/server/openejb-client/src/main/java/org/apache/openejb/ > client/Client.java > > b/server/openejb-client/src/main/java/org/apache/openejb/ > > client/Client.java > > index d017e7f..837a2ea 100644 > > --- a/server/openejb-client/src/main/java/org/apache/openejb/ > > client/Client.java > > +++ b/server/openejb-client/src/main/java/org/apache/openejb/ > > client/Client.java > > @@ -405,23 +405,6 @@ public class Client { > > } > > > > if (null != in) { > > - > > - // consume anything left in the buffer if we're running > > in http(s) mode > > - if (HttpConnectionFactory.HttpConnection.class. > isInstance(conn)) > > { > > - final HttpConnectionFactory.HttpConnection > > httpConnection = HttpConnectionFactory.HttpConnection.class.cast(conn); > > - if ("https".equalsIgnoreCase( > httpConnection.getURI().getScheme())) > > { > > - > > - try { > > - int read = 0; > > - while (read > -1) { > > - read = in.read(); > > - } > > - } catch (Throwable e) { > > - // ignore > > - } > > - } > > - } > > - > > try { > > in.close(); > > } catch (final Throwable e) { > > > > > > > -- > Jonathan Gallimore > http://twitter.com/jongallimore > http://www.tomitribe.com >
