[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openejb-development
Subject:    Re: tomee git commit: useless code since done in the connection close
From:       Romain Manni-Bucau <rmannibucau () gmail ! com>
Date:       2016-08-29 21:22:51
Message-ID: CACLE=7NviVntZgBmegkNoZZmKijJRdAmYt7=jC_PwP13P0B85g () mail ! gmail ! com
[Download RAW message or body]


+1, doesnt make sense to do both to me, in particular with our pooling
abstraction

Le 29 août 2016 21:42, "Jonathan Gallimore" <jgallimore@tomitribe.com> 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, <rmannibucau@apache.org> 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 <rmannibucau@gmail.com>
> > Authored: Mon Aug 29 11:22:51 2016 +0200
> > Committer: Romain manni-Bucau <rmannibucau@gmail.com>
> > 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
>


[prev in list] [next in list] [prev in thread] [next in thread]