Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers
> On 8 Jan 2020, at 05:12, Anuraag Agrawal wrote:
>
> Hi Chris,
>
> Happy new year - sorry for the long delay. I have made an updated patch
> fixing the comment (missing comma), bumping MAX_STR_LEN, and tweaking the
> exception behavior.
>
> Basically, as you pointed out loadDNSConfig itself does not throw any
> exception. The getAdapters function will throw a) OutOfMemoryError if native
> memory allocation failed or b) Error if the Windows SDK function randomly
> failed. I have gone ahead and added code to check the exception thrown when
> getAdapters failed and propagate it if it is OOME or clear it otherwise. This
> should be the same behavior as the old version of loadDNSConfig.
...
> -if (adapterP) {
> -free(adapterP);
> +free(adapters);
> +} else {
> +th = (*env)->ExceptionOccurred(env);
> +if (th != NULL) {
> +// We only throw errors if not able to allocate memory, otherwise
> +// ignore ones related to Windows API usage itself.
> +(*env)->ExceptionClear(env);
> +if ((*env)->IsInstanceOf(env, th, outOfMemoryErrCl)) {
> +(*env)->Throw(env, th);
> +return STS_ERROR;
> +}
> +}
I think that it would be preferable to NOT clear the pending exception, and
just allow it to propagate up.
-Chris.
Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers
Hi Chris,
Ah ok - I was worried it might be a backwards incompatible change, but I
guess these are internal classes anyways. I have updated the patch,
removing the exception handling logic so the errors propagate up.
Inline patch follows
diff --git
a/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
b/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
index 2250b3158e..c3ebe09776 100644
---
a/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
+++
b/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
@@ -25,9 +25,9 @@
package sun.net.dns;
+import java.util.ArrayList;
import java.util.List;
-import java.util.LinkedList;
-import java.util.StringTokenizer;
+import java.util.concurrent.TimeUnit;
/*
* An implementation of sun.net.ResolverConfiguration for Windows.
@@ -50,30 +50,55 @@ public class ResolverConfigurationImpl
// Cache timeout (120 seconds) - should be converted into property
// or configured as preference in the future.
-private static final int TIMEOUT = 12;
+private static final long TIMEOUT_NANOS =
TimeUnit.SECONDS.toNanos(120);
// DNS suffix list and name servers populated by native method
private static String os_searchlist;
private static String os_nameservers;
// Cached lists
-private static LinkedList searchlist;
-private static LinkedList nameservers;
-
-// Parse string that consists of token delimited by space or commas
-// and return LinkedHashMap
-private LinkedList stringToList(String str) {
-LinkedList ll = new LinkedList<>();
-
-// comma and space are valid delimiters
-StringTokenizer st = new StringTokenizer(str, ", ");
-while (st.hasMoreTokens()) {
-String s = st.nextToken();
-if (!ll.contains(s)) {
-ll.add(s);
+private static ArrayList searchlist;
+private static ArrayList nameservers;
+
+// Parse string that consists of token delimited by comma
+// and return ArrayList. Refer to ResolverConfigurationImpl.c and
+// strappend to see how the string is created.
+private ArrayList stringToList(String str) {
+// String is delimited by comma.
+String[] tokens = str.split(",");
+ArrayList l = new ArrayList<>(tokens.length);
+for (String s : tokens) {
+if (!l.contains(s)) {
+l.add(s);
}
}
-return ll;
+l.trimToSize();
+return l;
+}
+
+// Parse string that consists of token delimited by comma
+// and return ArrayList. Refer to ResolverConfigurationImpl.c and
+// strappend to see how the string is created.
+// In addition to splitting the string, converts IPv6 addresses to
+// BSD-style.
+private ArrayList addressesToList(String str) {
+// String is delimited by comma
+String[] tokens = str.split(",");
+ArrayList l = new ArrayList<>(tokens.length);
+
+for (String s : tokens) {
+if (!s.isEmpty()) {
+if (s.indexOf(':') >= 0 && s.charAt(0) != '[') {
+// Not BSD style
+s = '[' + s + ']';
+}
+if (!l.contains(s)) {
+l.add(s);
+}
+}
+}
+l.trimToSize();
+return l;
}
// Load DNS configuration from OS
@@ -81,28 +106,34 @@ public class ResolverConfigurationImpl
private void loadConfig() {
assert Thread.holdsLock(lock);
-// if address have changed then DNS probably changed as well;
-// otherwise check if cached settings have expired.
-//
+// A change in the network address of the machine usually indicates
+// a change in DNS configuration too so we always refresh the
config
+// after such a change.
if (changed) {
changed = false;
} else {
+// Otherwise we refresh if TIMEOUT_NANOS has passed since last
+// load.
if (lastRefresh >= 0) {
-long currTime = System.currentTimeMillis();
-if ((currTime - lastRefresh) < TIMEOUT) {
+long currTime = System.nanoTime();
+if ((currTime - lastRefresh) < TIMEOUT_NANOS) {
return;
}
}
}
-// load DNS configuration, update timestamp, create
-// new HashMaps from the loaded configuration
-//
+// Native code that uses Windows API to find out the DNS server
+// addresses and search suffixes. It builds a comma-delimited
string
+// of nameservers and domain suffixes and sets them to the static
+// os_nameservers and os_searchlist. We then split these into Java
+// Lists here.
loadDNSconfig0();
-lastRefresh = System.currentTimeMillis();
+// Re
Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers
Hi Anuraag,
I've uploaded your latest patch to the following location:
http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/04
The local Windows build and the acquired configuration are good.
I will run you patch through our CI system and will update this thread
once I get the results.
Kind Regards,
Aleksei
On 08/01/2020 15:50, Anuraag Agrawal wrote:
Hi Chris,
Ah ok - I was worried it might be a backwards incompatible change, but
I guess these are internal classes anyways. I have updated the patch,
removing the exception handling logic so the errors propagate up.
Inline patch follows
diff --git
a/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
b/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
index 2250b3158e..c3ebe09776 100644
---
a/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
+++
b/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
@@ -25,9 +25,9 @@
package sun.net.dns;
+import java.util.ArrayList;
import java.util.List;
-import java.util.LinkedList;
-import java.util.StringTokenizer;
+import java.util.concurrent.TimeUnit;
/*
* An implementation of sun.net.ResolverConfiguration for Windows.
@@ -50,30 +50,55 @@ public class ResolverConfigurationImpl
// Cache timeout (120 seconds) - should be converted into property
// or configured as preference in the future.
- private static final int TIMEOUT = 12;
+ private static final long TIMEOUT_NANOS =
TimeUnit.SECONDS.toNanos(120);
// DNS suffix list and name servers populated by native method
private static String os_searchlist;
private static String os_nameservers;
// Cached lists
- private static LinkedList searchlist;
- private static LinkedList nameservers;
-
- // Parse string that consists of token delimited by space or commas
- // and return LinkedHashMap
- private LinkedList stringToList(String str) {
- LinkedList ll = new LinkedList<>();
-
- // comma and space are valid delimiters
- StringTokenizer st = new StringTokenizer(str, ", ");
- while (st.hasMoreTokens()) {
- String s = st.nextToken();
- if (!ll.contains(s)) {
- ll.add(s);
+ private static ArrayList searchlist;
+ private static ArrayList nameservers;
+
+ // Parse string that consists of token delimited by comma
+ // and return ArrayList. Refer to ResolverConfigurationImpl.c and
+ // strappend to see how the string is created.
+ private ArrayList stringToList(String str) {
+ // String is delimited by comma.
+ String[] tokens = str.split(",");
+ ArrayList l = new ArrayList<>(tokens.length);
+ for (String s : tokens) {
+ if (!l.contains(s)) {
+ l.add(s);
}
}
- return ll;
+ l.trimToSize();
+ return l;
+ }
+
+ // Parse string that consists of token delimited by comma
+ // and return ArrayList. Refer to ResolverConfigurationImpl.c and
+ // strappend to see how the string is created.
+ // In addition to splitting the string, converts IPv6 addresses to
+ // BSD-style.
+ private ArrayList addressesToList(String str) {
+ // String is delimited by comma
+ String[] tokens = str.split(",");
+ ArrayList l = new ArrayList<>(tokens.length);
+
+ for (String s : tokens) {
+ if (!s.isEmpty()) {
+ if (s.indexOf(':') >= 0 && s.charAt(0) != '[') {
+ // Not BSD style
+ s = '[' + s + ']';
+ }
+ if (!l.contains(s)) {
+ l.add(s);
+ }
+ }
+ }
+ l.trimToSize();
+ return l;
}
// Load DNS configuration from OS
@@ -81,28 +106,34 @@ public class ResolverConfigurationImpl
private void loadConfig() {
assert Thread.holdsLock(lock);
- // if address have changed then DNS probably changed as well;
- // otherwise check if cached settings have expired.
- //
+ // A change in the network address of the machine usually
indicates
+ // a change in DNS configuration too so we always refresh the
config
+ // after such a change.
if (changed) {
changed = false;
} else {
+ // Otherwise we refresh if TIMEOUT_NANOS has passed since
last
+ // load.
if (lastRefresh >= 0) {
- long currTime = System.currentTimeMillis();
- if ((currTime - lastRefresh) < TIMEOUT) {
+ long currTime = System.nanoTime();
+ if ((currTime - lastRefresh) < TIMEOUT_NANOS) {
return;
}
}
}
- // load DNS configuration, update timestamp, create
- // new HashMaps from the loaded configuration
- //
+ // Native code that uses Windows
Re: [PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers
Got the testing results: the CI is happy with the last patch - no JNDI
test failures observed
Kind Regards,
Aleksei
On 08/01/2020 18:23, Aleks Efimov wrote:
Hi Anuraag,
I've uploaded your latest patch to the following location:
http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/04
The local Windows build and the acquired configuration are good.
I will run you patch through our CI system and will update this thread
once I get the results.
Kind Regards,
Aleksei
On 08/01/2020 15:50, Anuraag Agrawal wrote:
Hi Chris,
Ah ok - I was worried it might be a backwards incompatible change,
but I guess these are internal classes anyways. I have updated the
patch, removing the exception handling logic so the errors propagate up.
Inline patch follows
diff --git
a/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
b/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
index 2250b3158e..c3ebe09776 100644
---
a/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
+++
b/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
@@ -25,9 +25,9 @@
package sun.net.dns;
+import java.util.ArrayList;
import java.util.List;
-import java.util.LinkedList;
-import java.util.StringTokenizer;
+import java.util.concurrent.TimeUnit;
/*
* An implementation of sun.net.ResolverConfiguration for Windows.
@@ -50,30 +50,55 @@ public class ResolverConfigurationImpl
// Cache timeout (120 seconds) - should be converted into property
// or configured as preference in the future.
- private static final int TIMEOUT = 12;
+ private static final long TIMEOUT_NANOS =
TimeUnit.SECONDS.toNanos(120);
// DNS suffix list and name servers populated by native method
private static String os_searchlist;
private static String os_nameservers;
// Cached lists
- private static LinkedList searchlist;
- private static LinkedList nameservers;
-
- // Parse string that consists of token delimited by space or commas
- // and return LinkedHashMap
- private LinkedList stringToList(String str) {
- LinkedList ll = new LinkedList<>();
-
- // comma and space are valid delimiters
- StringTokenizer st = new StringTokenizer(str, ", ");
- while (st.hasMoreTokens()) {
- String s = st.nextToken();
- if (!ll.contains(s)) {
- ll.add(s);
+ private static ArrayList searchlist;
+ private static ArrayList nameservers;
+
+ // Parse string that consists of token delimited by comma
+ // and return ArrayList. Refer to ResolverConfigurationImpl.c and
+ // strappend to see how the string is created.
+ private ArrayList stringToList(String str) {
+ // String is delimited by comma.
+ String[] tokens = str.split(",");
+ ArrayList l = new ArrayList<>(tokens.length);
+ for (String s : tokens) {
+ if (!l.contains(s)) {
+ l.add(s);
}
}
- return ll;
+ l.trimToSize();
+ return l;
+ }
+
+ // Parse string that consists of token delimited by comma
+ // and return ArrayList. Refer to ResolverConfigurationImpl.c and
+ // strappend to see how the string is created.
+ // In addition to splitting the string, converts IPv6 addresses to
+ // BSD-style.
+ private ArrayList addressesToList(String str) {
+ // String is delimited by comma
+ String[] tokens = str.split(",");
+ ArrayList l = new ArrayList<>(tokens.length);
+
+ for (String s : tokens) {
+ if (!s.isEmpty()) {
+ if (s.indexOf(':') >= 0 && s.charAt(0) != '[') {
+ // Not BSD style
+ s = '[' + s + ']';
+ }
+ if (!l.contains(s)) {
+ l.add(s);
+ }
+ }
+ }
+ l.trimToSize();
+ return l;
}
// Load DNS configuration from OS
@@ -81,28 +106,34 @@ public class ResolverConfigurationImpl
private void loadConfig() {
assert Thread.holdsLock(lock);
- // if address have changed then DNS probably changed as well;
- // otherwise check if cached settings have expired.
- //
+ // A change in the network address of the machine usually
indicates
+ // a change in DNS configuration too so we always refresh
the config
+ // after such a change.
if (changed) {
changed = false;
} else {
+ // Otherwise we refresh if TIMEOUT_NANOS has passed
since last
+ // load.
if (lastRefresh >= 0) {
- long currTime = System.currentTimeMillis();
- if ((currTime - lastRefresh) < TIMEOUT) {
+ long currTime = System.nanoTime();
+ if ((currTime - lastRefresh) < TIMEOUT_NANOS) {
return;
}
}
}
-
