Re: Crash problem in net.dll

[Michael McMahon]

Just to be clear, I will run the tests on the fix and push the change
assuming everything is ok. There is a webrev here
http://cr.openjdk.java.net/~michaelm/7084560/webrev.1/

Thanks,
Michael.

On 29/08/11 15:33, Michael McMahon wrote:

Jonathan,

Bug id 7084560 has been created to track this. That fix looks fine
to me.

Thanks,
Michael.

On 29/08/11 10:20, Jonathan Lu wrote:

Hi Michael,

Thanks for reminding, I checked the source under src/windows folder and found two more places to change.

Here's the updated patch:

diff -r 9b8c96f96a0f src/windows/native/java/net/NetworkInterface.c

--- a/src/windows/native/java/net/NetworkInterface.c Mon Jun 27 13:21:34 2011 -0700 +++ b/src/windows/native/java/net/NetworkInterface.c Mon Aug 29 17:02:56 2011 +0800

@@ -504,8 +504,7 @@
      */
     if (netaddrCount < 0) {
         netaddrCount = enumAddresses_win(env, ifs, &netaddrP);
-        if ((*env)->ExceptionOccurred(env)) {
-            free_netaddr(netaddrP);
+        if (netaddrCount == -1) {
             return NULL;
         }
     }

diff -r 9b8c96f96a0f src/windows/native/java/net/NetworkInterface_winXP.c --- a/src/windows/native/java/net/NetworkInterface_winXP.c Mon Jun 27 13:21:34 2011 -0700 +++ b/src/windows/native/java/net/NetworkInterface_winXP.c Mon Aug 29 17:02:56 2011 +0800

@@ -194,8 +194,7 @@
     while (curr != NULL) {
         netaddr *netaddrP;
         ret = enumAddresses_win(env, curr, &netaddrP);
-        if ((*env)->ExceptionOccurred(env)) {
-            free_netaddr(netaddrP);
+        if (ret == -1) {
             return -1;
         }
         curr->addrs = netaddrP;
@@ -449,8 +448,7 @@
      */
     if (netaddrCount < 0) {
         netaddrCount = enumAddresses_win(env, ifs, &netaddrP);
-        if ((*env)->ExceptionOccurred(env)) {
-            free_netaddr(netaddrP);
+        if (count == -1) {
             return NULL;
         }
     }

And from my point of view, such scenario may also appear in other parts, maybe a more thorough check is needed for all modules.

Regards!
- Jonathan Lu

On 08/29/2011 04:40 PM, Michael McMahon wrote:

Jonathan,

I think the change looks reasonable. But, we should make the equivalent

change at the other site where enumAddresses_win() is called in the same file.

- Michael.

On 29/08/11 09:14, Jonathan Lu wrote:

Hello everybody,

I got one crash issue on OpenJDK7 windows build.

I captured the stack trace of such a crash, which happens intermittently on my 64bit Windows 2008 server. But unfortunately I have no simple test case to reproduce this problem.

RtlInterlockedFlushSList+0x2ea (0x779F2A7F [ntdll+0x32a7f])
RtlInterlockedFlushSList+0x572 (0x779F2D07 [ntdll+0x32d07])
RtlInterlockedFlushSList+0x45d (0x779F2BF2 [ntdll+0x32bf2])
HeapFree+0x14 (0x755A14D1 [kernel32+0x114d1])
free+0x1c (0x7284016A [msvcr100+0x1016a])
free_netaddr+0x11 (networkinterface.c:107, 0x725A12AC [net+0x12ac])

getAllInterfacesAndAddresses+0xb6 (networkinterface_winxp.c:199, 0x725AB4C8 [net+0xb4c8]) Java_java_net_NetworkInterface_getAll_XP+0x12 (networkinterface_winxp.c:693, 0x725AB7AB [net+0xb7ab])

From the code at src/windows/native/java/net/NetworkInterface_winXP.c:195, I believe it is the dangling pointers that caused this problem. The uninitialized pointer netaddrP is exceptionally freed if the call to Windows API "GetIpAddrTable()" fails.

So here's one proposed solution for this issue, can anybody please help to take a look?

diff -r 9b8c96f96a0f src/windows/native/java/net/NetworkInterface_winXP.c --- a/src/windows/native/java/net/NetworkInterface_winXP.c Mon Jun 27 13:21:34 2011 -0700 +++ b/src/windows/native/java/net/NetworkInterface_winXP.c Mon Aug 29 14:59:09 2011 +0800

@@ -194,8 +194,7 @@
     while (curr != NULL) {
         netaddr *netaddrP;
         ret = enumAddresses_win(env, curr, &netaddrP);
-        if ((*env)->ExceptionOccurred(env)) {
-            free_netaddr(netaddrP);
+        if (ret == -1) {
             return -1;
         }
         curr->addrs = netaddrP;

Best regards!

Jonathan Lu