[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[cp-patches] FYI: java-net native cleanup (and Socket timeouts)
|
From: |
Mark Wielaard |
|
Subject: |
[cp-patches] FYI: java-net native cleanup (and Socket timeouts) |
|
Date: |
Mon, 11 Jul 2005 20:20:43 +0200 |
Hi,
This cleans up a lot of the java-net native code. In several places we
were masking the actual exception by throwing a new exception. In other
places we did cleanup before we used the result of perror() which meant
that the error message was often bogus. We weren't checking whether
system calls were interrupted everywhere (most noticeable by failing
connect() calls). We had a completely bogus implementation of
TARGET_NATIVE_NETWORK_SOCKET_SET_OPTION_SO_TIMEOUT and
TARGET_NATIVE_NETWORK_SOCKET_GET_OPTION_SO_TIMEOUT which used an int
instead of a struct timeval. And worse we were ignoring the failure in
the jni code. So socket timeouts didn't work at all.
2005-07-10 Mark Wielaard <address@hidden>
* native/jni/java-net/gnu_java_net_PlainDatagramSocketImpl.c:
Whenever an ExceptionOccurred just return to throw it, don't mask.
* native/jni/java-net/javanet.c (_javanet_get_netaddr): Check for
NULL addr.
(_javanet_create): Explicitly close socket on failure.
(_javanet_close): Save error message and retry closing when
interrupted before throwing exception.
(_javanet_connect): Keep retrying connect after system call
interrupted. First construct exception before cleanup.
(_javanet_bind): Save error string for exception.
(_javanet_accept): Explicitly close socket on failure.
(_javanet_recvfrom): Throw SocketTimeoutException when timed out.
(_javanet_sendto): Send all data even when interrupted.
(_javanet_set_option): Don't ignore error when setting SO_TIMEOUT.
* native/target/generic/target_generic_network.h
(TARGET_NATIVE_NETWORK_SOCKET_SET_OPTION_SO_TIMEOUT): Use timeval for
setsockopt.
(TARGET_NATIVE_NETWORK_SOCKET_GET_OPTION_SO_TIMEOUT): Likewise for
getsockopt.
This fixes several Mauve failures and gives no new regressions. Some of
my example applications also work much better.
Committed,
Mark
? native/target/generic/semantic.cache
Index: native/jni/java-net/gnu_java_net_PlainDatagramSocketImpl.c
===================================================================
RCS file:
/cvsroot/classpath/classpath/native/jni/java-net/gnu_java_net_PlainDatagramSocketImpl.c,v
retrieving revision 1.11
diff -u -r1.11 gnu_java_net_PlainDatagramSocketImpl.c
--- native/jni/java-net/gnu_java_net_PlainDatagramSocketImpl.c 2 Jul 2005
20:32:55 -0000 1.11
+++ native/jni/java-net/gnu_java_net_PlainDatagramSocketImpl.c 11 Jul 2005
18:00:24 -0000
@@ -207,7 +207,9 @@
}
arr = (*env)->CallObjectMethod (env, packet, mid);
- if ((arr == NULL) || (*env)->ExceptionOccurred (env))
+ if ((*env)->ExceptionOccurred (env))
+ return;
+ if (arr == NULL)
{
JCL_ThrowException (env, IO_EXCEPTION, "Internal error: call getData");
return;
@@ -223,11 +225,7 @@
offset = (*env)->CallIntMethod (env, packet, mid);
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION,
- "Internal error: call getOffset");
- return;
- }
+ return;
DBG ("PlainDatagramSocketImpl.receive(): Got the offset\n");
@@ -241,16 +239,15 @@
maxlen = (*env)->GetIntField (env, packet, fid);
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION, "Internal error: call length");
- return;
- }
+ return;
/* Receive the packet */
/* should we try some sort of validation on the length? */
bytes_read =
_javanet_recvfrom (env, obj, arr, offset, maxlen, &addr, &port);
- if ((bytes_read == -1) || (*env)->ExceptionOccurred (env))
+ if ((*env)->ExceptionOccurred (env))
+ return;
+ if (bytes_read == -1)
{
JCL_ThrowException (env, IO_EXCEPTION, "Internal error: receive");
return;
@@ -292,11 +289,7 @@
addr_obj = (*env)->CallStaticObjectMethod (env, addr_cls, mid, ip_str_obj);
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION,
- "Internal error: call getByName");
- return;
- }
+ return;
mid = (*env)->GetMethodID (env, cls, "setAddress",
"(Ljava/net/InetAddress;)V");
@@ -308,11 +301,7 @@
(*env)->CallVoidMethod (env, packet, mid, addr_obj);
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION,
- "Internal error: call setAddress");
- return;
- }
+ return;
DBG ("PlainDatagramSocketImpl.receive(): Stored the address\n");
@@ -326,10 +315,7 @@
(*env)->CallVoidMethod (env, packet, mid, port);
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION, "Internal error: call setPort");
- return;
- }
+ return;
DBG ("PlainDatagramSocketImpl.receive(): Stored the port\n");
@@ -343,10 +329,7 @@
(*env)->SetIntField (env, packet, fid, bytes_read);
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION, "Internal error: call length");
- return;
- }
+ return;
DBG ("PlainDatagramSocketImpl.receive(): Stored the length\n");
#else /* not WITHOUT_NETWORK */
@@ -372,20 +355,13 @@
netAddress = _javanet_get_netaddr (env, addr);
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION,
- "Internal error: get network address");
- return;
- }
+ return;
DBG ("PlainDatagramSocketImpl.sendto(): have addr\n");
_javanet_sendto (env, obj, buf, offset, len, netAddress, port);
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION, "Internal error: send data");
- return;
- }
+ return;
DBG ("PlainDatagramSocketImpl.sendto(): finished\n");
#else /* not WITHOUT_NETWORK */
@@ -411,17 +387,11 @@
netAddress = _javanet_get_netaddr (env, addr);
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION, "Internal error");
- return;
- }
+ return;
fd = _javanet_get_int_field (env, obj, "native_fd");
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION, "Internal error");
- return;
- }
+ return;
DBG ("PlainDatagramSocketImpl.join(): have native fd\n");
@@ -459,17 +429,11 @@
netAddress = _javanet_get_netaddr (env, addr);
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION, "Internal error");
- return;
- }
+ return;
fd = _javanet_get_int_field (env, obj, "native_fd");
if ((*env)->ExceptionOccurred (env))
- {
- JCL_ThrowException (env, IO_EXCEPTION, "Internal error");
- return;
- }
+ return;
DBG ("PlainDatagramSocketImpl.leave(): have native fd\n");
Index: native/jni/java-net/javanet.c
===================================================================
RCS file: /cvsroot/classpath/classpath/native/jni/java-net/javanet.c,v
retrieving revision 1.22
diff -u -r1.22 javanet.c
--- native/jni/java-net/javanet.c 2 Jul 2005 20:32:55 -0000 1.22
+++ native/jni/java-net/javanet.c 11 Jul 2005 18:00:24 -0000
@@ -377,6 +377,13 @@
DBG ("_javanet_get_netaddr(): Entered _javanet_get_netaddr\n");
+ if (addr == NULL)
+ {
+ JCL_ThrowException (env, "java/lang/NullPointerException",
+ "Null address");
+ return 0;
+ }
+
/* Call the getAddress method on the object to retrieve the IP address */
cls = (*env)->GetObjectClass (env, addr);
if (cls == NULL)
@@ -473,6 +480,22 @@
else
_javanet_set_int_field (env, this, "gnu/java/net/PlainDatagramSocketImpl",
"native_fd", fd);
+
+ if ((*env)->ExceptionOccurred (env))
+ {
+ /* Try to make sure we close the socket since close() won't work. */
+ do
+ {
+ TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
+ if (result != TARGET_NATIVE_OK
+ && (TARGET_NATIVE_LAST_ERROR ()
+ != TARGET_NATIVE_ERROR_INTERRUPT_FUNCTION_CALL))
+ return;
+ }
+ while (result != TARGET_NATIVE_OK);
+ return;
+ }
+
#else /* not WITHOUT_NETWORK */
#endif /* not WITHOUT_NETWORK */
}
@@ -489,6 +512,7 @@
#ifndef WITHOUT_NETWORK
int fd;
int result;
+ int error = 0;
assert (env != NULL);
assert ((*env) != NULL);
@@ -497,14 +521,27 @@
if (fd == -1)
return;
- TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
-
if (stream)
_javanet_set_int_field (env, this, "gnu/java/net/PlainSocketImpl",
"native_fd", -1);
else
_javanet_set_int_field (env, this, "gnu/java/net/PlainDatagramSocketImpl",
"native_fd", -1);
+ do
+ {
+ TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
+ if (result != TARGET_NATIVE_OK)
+ {
+ /* Only throw an error when a "real" error occurs. */
+ error = TARGET_NATIVE_LAST_ERROR ();
+ if (error != TARGET_NATIVE_ERROR_INTERRUPT_FUNCTION_CALL
+ && error != ENOTCONN && error != ECONNRESET && error != EBADF)
+ JCL_ThrowException (env, IO_EXCEPTION,
+ TARGET_NATIVE_LAST_ERROR_STRING ());
+ }
+ }
+ while (error == TARGET_NATIVE_ERROR_INTERRUPT_FUNCTION_CALL);
+
#else /* not WITHOUT_NETWORK */
#endif /* not WITHOUT_NETWORK */
}
@@ -548,13 +585,20 @@
DBG ("_javanet_connect(): Got native fd\n");
/* Connect up */
- TARGET_NATIVE_NETWORK_SOCKET_CONNECT (fd, netaddr, port, result);
- if (result != TARGET_NATIVE_OK)
+ do
{
- JCL_ThrowException (env, IO_EXCEPTION,
- TARGET_NATIVE_LAST_ERROR_STRING ());
- return;
+ TARGET_NATIVE_NETWORK_SOCKET_CONNECT (fd, netaddr, port, result);
+ if (result != TARGET_NATIVE_OK
+ && (TARGET_NATIVE_LAST_ERROR ()
+ != TARGET_NATIVE_ERROR_INTERRUPT_FUNCTION_CALL))
+ {
+ JCL_ThrowException (env, IO_EXCEPTION,
+ TARGET_NATIVE_LAST_ERROR_STRING ());
+ return;
+ }
}
+ while (result != TARGET_NATIVE_OK);
+
DBG ("_javanet_connect(): Connected successfully\n");
/* Populate instance variables */
@@ -562,15 +606,17 @@
result);
if (result != TARGET_NATIVE_OK)
{
- TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
JCL_ThrowException (env, IO_EXCEPTION,
TARGET_NATIVE_LAST_ERROR_STRING ());
+ /* We don't care whether this succeeds. close() will cleanup later. */
+ TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
return;
}
_javanet_create_localfd (env, this);
if ((*env)->ExceptionOccurred (env))
{
+ /* We don't care whether this succeeds. close() will cleanup later. */
TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
return;
}
@@ -580,6 +626,7 @@
local_port);
if ((*env)->ExceptionOccurred (env))
{
+ /* We don't care whether this succeeds. close() will cleanup later. */
TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
return;
}
@@ -589,9 +636,10 @@
remote_port, result);
if (result != TARGET_NATIVE_OK)
{
- TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
JCL_ThrowException (env, IO_EXCEPTION,
TARGET_NATIVE_LAST_ERROR_STRING ());
+ /* We don't care whether this succeeds. close() will cleanup later. */
+ TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
return;
}
@@ -605,6 +653,7 @@
}
if ((*env)->ExceptionOccurred (env))
{
+ /* We don't care whether this succeeds. close() will cleanup later. */
TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
return;
}
@@ -614,6 +663,7 @@
remote_port);
if ((*env)->ExceptionOccurred (env))
{
+ /* We don't care whether this succeeds. close() will cleanup later. */
TARGET_NATIVE_NETWORK_SOCKET_CLOSE (fd, result);
return;
}
@@ -698,16 +748,19 @@
octets[3], tmpaddr);
TARGET_NATIVE_NETWORK_SOCKET_BIND (fd, tmpaddr, port, result);
- (*env)->ReleaseByteArrayElements (env, arr, octets, 0);
-
if (result != TARGET_NATIVE_OK)
{
+ char *errorstr = TARGET_NATIVE_LAST_ERROR_STRING ();
+ (*env)->ReleaseByteArrayElements (env, arr, octets, 0);
+
JCL_ThrowException (env, BIND_EXCEPTION,
- TARGET_NATIVE_LAST_ERROR_STRING ());
+ errorstr);
return;
}
DBG ("_javanet_bind(): Past bind\n");
+ (*env)->ReleaseByteArrayElements (env, arr, octets, 0);
+
/* Update instance variables, specifically the local port number */
TARGET_NATIVE_NETWORK_SOCKET_GET_LOCAL_INFO (fd, local_address, local_port,
result);
@@ -816,7 +869,16 @@
if ((*env)->ExceptionOccurred (env))
{
- TARGET_NATIVE_NETWORK_SOCKET_CLOSE (newfd, result);
+ /* Try to make sure we close the socket since close() won't work. */
+ do
+ {
+ TARGET_NATIVE_NETWORK_SOCKET_CLOSE (newfd, result);
+ if (result != TARGET_NATIVE_OK
+ && (TARGET_NATIVE_LAST_ERROR ()
+ != TARGET_NATIVE_ERROR_INTERRUPT_FUNCTION_CALL))
+ return;
+ }
+ while (result != TARGET_NATIVE_OK);
return;
}
@@ -824,6 +886,7 @@
local_port, result);
if (result != TARGET_NATIVE_OK)
{
+ /* We don't care whether this succeeds. close() will cleanup later. */
TARGET_NATIVE_NETWORK_SOCKET_CLOSE (newfd, result);
JCL_ThrowException (env, IO_EXCEPTION,
TARGET_NATIVE_LAST_ERROR_STRING ());
@@ -833,6 +896,7 @@
_javanet_create_localfd (env, impl);
if ((*env)->ExceptionOccurred (env))
{
+ /* We don't care whether this succeeds. close() will cleanup later. */
TARGET_NATIVE_NETWORK_SOCKET_CLOSE (newfd, result);
return;
}
@@ -841,6 +905,7 @@
local_port);
if ((*env)->ExceptionOccurred (env))
{
+ /* We don't care whether this succeeds. close() will cleanup later. */
TARGET_NATIVE_NETWORK_SOCKET_CLOSE (newfd, result);
return;
}
@@ -849,15 +914,17 @@
remote_port, result);
if (result != TARGET_NATIVE_OK)
{
- TARGET_NATIVE_NETWORK_SOCKET_CLOSE (newfd, result);
JCL_ThrowException (env, IO_EXCEPTION,
TARGET_NATIVE_LAST_ERROR_STRING ());
+ /* We don't care whether this succeeds. close() will cleanup later. */
+ TARGET_NATIVE_NETWORK_SOCKET_CLOSE (newfd, result);
return;
}
_javanet_set_remhost (env, impl, remote_address);
if ((*env)->ExceptionOccurred (env))
{
+ /* We don't care whether this succeeds. close() will cleanup later. */
TARGET_NATIVE_NETWORK_SOCKET_CLOSE (newfd, result);
return;
}
@@ -866,6 +933,7 @@
remote_port);
if ((*env)->ExceptionOccurred (env))
{
+ /* We don't care whether this succeeds. close() will cleanup later. */
TARGET_NATIVE_NETWORK_SOCKET_CLOSE (newfd, result);
return;
}
@@ -945,15 +1013,21 @@
(TARGET_NATIVE_LAST_ERROR () ==
TARGET_NATIVE_ERROR_INTERRUPT_FUNCTION_CALL));
- (*env)->ReleaseByteArrayElements (env, buf, p, 0);
-
if (received_bytes == -1)
{
- JCL_ThrowException (env, IO_EXCEPTION,
- TARGET_NATIVE_LAST_ERROR_STRING ());
+ if (TARGET_NATIVE_LAST_ERROR () == EAGAIN)
+ JCL_ThrowException (env, "java/net/SocketTimeoutException", "Timeout");
+ else
+ JCL_ThrowException (env, IO_EXCEPTION,
+ TARGET_NATIVE_LAST_ERROR_STRING ());
+
+ /* Cleanup and return. */
+ (*env)->ReleaseByteArrayElements (env, buf, p, 0);
return 0;
}
+ (*env)->ReleaseByteArrayElements (env, buf, p, 0);
+
/* Handle return addr case */
if (addr != NULL)
{
@@ -1004,29 +1078,42 @@
if (p == NULL)
return;
- /* Send the data */
- if (addr == 0)
+ /* We must send all the data, so repeat till done. */
+ while (len > 0)
{
- DBG ("_javanet_sendto(): Sending....\n");
- TARGET_NATIVE_NETWORK_SOCKET_SEND (fd, p + offset, len, bytes_sent);
- }
- else
- {
- DBG ("_javanet_sendto(): Sending....\n");
- TARGET_NATIVE_NETWORK_SOCKET_SEND_WITH_ADDRESS_PORT (fd, p + offset,
- len, addr, port,
- bytes_sent);
+ /* Send the data */
+ if (addr == 0)
+ {
+ DBG ("_javanet_sendto(): Sending....\n");
+ TARGET_NATIVE_NETWORK_SOCKET_SEND (fd, p + offset, len, bytes_sent);
+ }
+ else
+ {
+ DBG ("_javanet_sendto(): Sending....\n");
+ TARGET_NATIVE_NETWORK_SOCKET_SEND_WITH_ADDRESS_PORT (fd, p + offset,
+ len, addr, port,
+ bytes_sent);
+ }
+
+ if (bytes_sent < 0)
+ {
+ if (TARGET_NATIVE_LAST_ERROR ()
+ != TARGET_NATIVE_ERROR_INTERRUPT_FUNCTION_CALL)
+ {
+ JCL_ThrowException (env, IO_EXCEPTION,
+ TARGET_NATIVE_LAST_ERROR_STRING ());
+ break;
+ }
+ }
+ else
+ {
+ len -= bytes_sent;
+ addr += bytes_sent;
+ }
}
(*env)->ReleaseByteArrayElements (env, buf, p, 0);
- /***** Do we need to check EINTR? */
- if (bytes_sent < 0)
- {
- JCL_ThrowException (env, IO_EXCEPTION,
- TARGET_NATIVE_LAST_ERROR_STRING ());
- return;
- }
#else /* not WITHOUT_NETWORK */
#endif /* not WITHOUT_NETWORK */
}
@@ -1139,9 +1226,9 @@
return;
TARGET_NATIVE_NETWORK_SOCKET_SET_OPTION_SO_TIMEOUT (fd, optval, result);
-#endif
- /* ignore errors and do not throw an exception. */
+#else
result = TARGET_NATIVE_OK;
+#endif
break;
case SOCKOPT_SO_SNDBUF:
@@ -1326,7 +1413,6 @@
TARGET_NATIVE_LAST_ERROR_STRING ());
return (0);
}
-
return (_javanet_create_integer (env, optval));
#else
JCL_ThrowException (env, SOCKET_EXCEPTION,
Index: native/target/generic/target_generic_network.h
===================================================================
RCS file:
/cvsroot/classpath/classpath/native/target/generic/target_generic_network.h,v
retrieving revision 1.13
diff -u -r1.13 target_generic_network.h
--- native/target/generic/target_generic_network.h 2 Jul 2005 20:32:55
-0000 1.13
+++ native/target/generic/target_generic_network.h 11 Jul 2005 18:00:24
-0000
@@ -1,5 +1,5 @@
/* target_generic_network.h - Native methods for network operations.
- Copyright (C) 1998, 2004 Free Software Foundation, Inc.
+ Copyright (C) 1998, 2004, 2005 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -668,9 +668,10 @@
#include <sys/socket.h>
#define
TARGET_NATIVE_NETWORK_SOCKET_SET_OPTION_SO_TIMEOUT(socketDescriptor,flag,result)
\
do { \
- int __value; \
+ struct timeval __value; \
\
- __value=flag; \
+ __value.tv_sec = flag / 1000; \
+ __value.tv_usec = (flag % 1000) * 1000; \
result=(setsockopt(socketDescriptor,SOL_SOCKET,SO_TIMEOUT,&__value,sizeof(__value))==0)?TARGET_NATIVE_OK:TARGET_NATIVE_ERROR;
\
} while (0)
#endif
@@ -682,7 +683,7 @@
* size - size of send buffer
* Output : result - TARGET_NATIVE_OK if no error occurred,
* TARGET_NATIVE_ERROR otherwise
-* Return : -
+* Return : -
* Side-effect: unknown
* Notes : -
\***********************************************************************/
@@ -989,7 +990,7 @@
#include <sys/socket.h>
#define
TARGET_NATIVE_NETWORK_SOCKET_GET_OPTION_SO_TIMEOUT(socketDescriptor,flag,result)
\
do { \
- int __value; \
+ struct timeval __value; \
socklen_t __len; \
\
flag=0; \
@@ -998,7 +999,7 @@
result=(getsockopt(socketDescriptor,SOL_SOCKET,SO_TIMEOUT,&__value,&__len)==0)?TARGET_NATIVE_OK:TARGET_NATIVE_ERROR;
\
if (result==TARGET_NATIVE_OK) \
{ \
- flag=__value; \
+ flag = (__value.tv_sec * 1000LL) + (__value.tv_usec / 1000LL); \
} \
} while (0)
#endif
signature.asc
Description: This is a digitally signed message part
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- [cp-patches] FYI: java-net native cleanup (and Socket timeouts),
Mark Wielaard <=