[8u] 8217606: LdapContext#reconnect always opens a new connection

[Hohensee, Paul]

You're right, my bad. I'd forgot that try-with-resources will close its argument if necessary, so http://cr.openjdk.java.net/~zgu/JDK-8217606-8u/webrev.01/ is correct.

Thanks,
Paul

On 7/13/20, 11:06 AM, "Zhengyu Gu" <zgu at redhat.com> wrote:

    Hi Paul,

    Thanks for reviewing.


    On 7/13/20 1:19 PM, Hohensee, Paul wrote:
    > There's an extra blank line in your version just before the try/catch,
    Fixed.

    and looks like you added a finally clause in
    BaseLdapServer.handleConnection().

    Original version has:

             ...
             // automatically when `socket.close()` is called
             beforeConnectionHandled(socket);
             try (socket) {
                 OutputStream out = socket.getOutputStream();
                 InputStream in = socket.getInputStream();
                 byte[] inBuffer = new byte[1024];
             ...

    8u complains about: try(socket), I converted to try/catch/finally. So,
    it is not a new addition.

    updated:  http://cr.openjdk.java.net/~zgu/JDK-8217606-8u/webrev.02/

    Thanks,

    -Zhengyu


    The latter is a good reliability addition, but I'd fix it in upstream
    and backport it separately.
    >
    > Thanks,
    > Paul
    >
    > On 7/13/20, 9:11 AM, "jdk8u-dev on behalf of Zhengyu Gu" <jdk8u-dev-retn at openjdk.java.net on behalf of zgu at redhat.com> wrote:
    >
    >      On 6/29/20 3:55 PM, Zhengyu Gu wrote:
    >      > Hi,
    >      >
    >      > I would like to backport this patch to 8u for Oracle 8u271 parity.
    >      >
    >      > The original patch does not apply cleanly.
    >      >
    >      > LdapCtx.java:
    >      > 8u does not have 'reconnect' member, so removing the variable lines do
    >      > not apply.
    >      >
    >      > Reconnect.java:
    >      > Uses try-catch-with-resource statement, which does not exist in 8u.
    >      > Converted to try-catch-finally.
    >      try-with-resource here works find.
    >
    >      Updated: http://cr.openjdk.java.net/~zgu/JDK-8217606-8u/webrev.01/
    >
    >      Thanks,
    >
    >      -Zhengyu
    >
    >      > Uses 'var' language feature, which does not exist in 8u. Converted
    >      > accordingly.
    >      >
    >      > BaseLdapServer.java:
    >      > The same issues as Reconnect.java, fixed accordingly.
    >      > Uses System.Logger, which does not exist in 8u. Converted to
    >      > java.util.logging.Logger.
    >      >
    >      > Original bug: https://bugs.openjdk.java.net/browse/JDK-8217606
    >      > Original patch: https://hg.openjdk.java.net/jdk/jdk/rev/6717d7e59db4
    >      >
    >      > 8u webrev: http://cr.openjdk.java.net/~zgu/JDK-8217606-8u/webrev.00/
    >      >
    >      > Test:
    >      >    test/com/sun/jndi/ldap/LdapCtx/Reconnect.java
    >      >
    >      > Thanks,
    >      >
    >      > -Zhengyu
    >      >
    >      >
    >      >
    >      >
    >
    >