[11u] RFR (S): 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Lindenmaier, Goetz]

Looks good now, thanks!

Best regards,
  Goetz.

-----Original Message-----
From: Langer, Christoph <christoph.langer at sap.com> 
Sent: Montag, 31. August 2020 17:14
To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; jdk-updates-dev at openjdk.java.net
Subject: RE: [11u] RFR (S): 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

Hi Goetz,

good catch. Seems like I overlooked that one.

Here's an updated webrev: http://cr.openjdk.java.net/~clanger/webrevs/8151678.11u.1/

Best regards
Christoph


> -----Original Message-----
> From: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Sent: Montag, 31. August 2020 16:07
> To: Langer, Christoph <christoph.langer at sap.com>; jdk-updates-
> dev at openjdk.java.net
> Subject: RE: [11u] RFR (S): 8151678:
> com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on
> DeadServerNoTimeoutTest is incorrect
> 
> Hi Christoph,
> 
> I had a look at your change.
> One question:
> 
> Why do you use size() and not emtpy() here?
> 
> LdapDnsProviderService.java:
> @@ -99,14 +101,16 @@
>      LdapDnsProviderResult lookupEndpoints(String url, Hashtable<?,?> env)
>          throws NamingException
>      {
> -        Iterator<LdapDnsProvider> iterator = providers.iterator();
> -        Hashtable<?, ?> envCopy = new Hashtable<>(env);
>          LdapDnsProviderResult result = null;
> +        Hashtable<?, ?> envCopy = new Hashtable<>(env);
> 
> -        while (result == null && iterator.hasNext()) {
> -            result = iterator.next().lookupEndpoints(url, envCopy)
> -                    .filter(r -> r.getEndpoints().size() > 0)
> -                    .orElse(null);
> +        synchronized (LOCK) {
> +            Iterator<LdapDnsProvider> iterator = providers.iterator();
> +            while (result == null && iterator.hasNext()) {
> +                result = iterator.next().lookupEndpoints(url, envCopy)
> +                        .filter(r -> r.getEndpoints().size() > 0)
> <=======
> +                        .orElse(null);
> +            }
>          }
> 
>          return result;
> 
> Besides that it looks good.
> 
> Best regards,
>   Goetz.
> 
> 
> 
> 
> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-retn at openjdk.java.net> On
> Behalf Of Langer, Christoph
> Sent: Montag, 31. August 2020 11:31
> To: jdk-updates-dev at openjdk.java.net
> Subject: [CAUTION] [11u] RFR (S): 8151678:
> com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on
> DeadServerNoTimeoutTest is incorrect
> 
> Hi,
> 
> may I please have a review for the backport of test fix JDK-8151678
> "com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on
> DeadServerNoTimeoutTest is incorrect" as follow-up for JDK-8160768 (Add
> capability to custom resolve host/domain names within the default JNDI
> LDAP provider).
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8151678
> Original change: https://hg.openjdk.java.net/jdk/jdk/rev/1def54255e93
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8151678.11u.0/
> 
> I had the following Rejects:
> src/java.naming/share/classes/com/sun/jndi/ldap/DefaultLdapDnsProvider.j
> ava:
>     needed manual resolve, also copyright year was different
> LdapDnsProviderService.java:
>     File is in a different location in 11u, so I needed to manually resolve it:
> 
> java.naming/share/classes/com/sun/jndi/ldap/LdapDnsProviderService.java
> ->
> jdk.naming.ldap/share/classes/com/sun/jndi/ldap/dns/LdapDnsProviderSer
> vice.java
> ProblemList.txt:
>     Needed a manual resolve
> 
> Thanks
> Christoph