SUGGESTED FIX
Fix for 1.3.1 code base:
------- PlainSocketImpl.java -------
*** /tmp/sccs.J7aGol Wed Aug 13 12:28:25 2003
--- PlainSocketImpl.java Wed Aug 13 12:28:24 2003
***************
*** 508,513 ****
--- 508,516 ----
synchronized(fdLock) {
if (fd != null) {
if (fdUseCount == 0) {
+ if (closePending) {
+ return;
+ }
closePending = true;
socketClose(false);
fd = null;
###@###.### 2003-08-13
--- SocketInputStream.c Mon Oct 13 01:20:16 2003
*** 50,60 ****
char *bufP;
char BUF[MAX_BUFFER_LEN];
jobject fdObj = (*env)->GetObjectField(env, this, sis_fdID);
! jint fd, timeout;
jobject impl = (*env)->GetObjectField(env, this, sis_implID);
jint datalen, nread;
--- 50,60 ----
char *bufP;
char BUF[MAX_BUFFER_LEN];
jobject fdObj = (*env)->GetObjectField(env, this, sis_fdID);
! jint fd, timeout, newfd;
jobject impl = (*env)->GetObjectField(env, this, sis_implID);
jint datalen, nread;
*** 123,132 ****
--- 123,139 ----
if (bufP != BUF) {
free(bufP);
}
return -1;
}
+
+ /*check if the socket has been closed while we were in timeout*/
+ newfd = (*env)->GetIntField(env, fdObj, IO_fd_fdID);
+ if (newfd == -1) {
+ NET_ThrowSocketException(env, "Socket Closed");
+ return -1;
+ }
}
nread = JVM_Recv(fd, bufP, len, 0);
if (nread > 0) {
(*env)->SetByteArrayRegion(env, data, off, nread, (jbyte *)bufP);
} else {
###@###.### 2003-10-28
|
EVALUATION
I modified the testcase to read 3 bytes in the second read() so as to distinguish the two 'read's. I analysed the system level calls made by the testcase using 'truss'. This is what is happening at the system level:
1336/11: read(83, "\0010203", 4) = 4
...
1336/13: close(83) = 0
...
1336/6: accept(6, 0xEE58153C, 0xEE58154C, 1) = 83
...
1336/11: read(83, "\00102", 3) = 3
Thread 11 reads 4 bytes in the first read call on the socket with fd 83 and notifies other thread to close this socket and proceeds to read 3 more bytes. Meanwhile Thread 13 closes that fd and creates another socket with same fd. Now Thread 11 reads 3 bytes on fd 83 as this is the first read after creation of fd 83 again.
SocketInputStream class has an instance variable 'impl' of type PlainSocketImpl which contains an instance variable of type FileDescriptor (fd). SocketInputStream has methods to read and close the stream which in turn call the impl's methods to get the fd in read and set the fd to null in close.
Now here what could be happening is: While one thread is in the second read blocked at timeout, another thread closes the stream and sets the 'fd' of PlainSocketImpl to NULL and one another thread creates one more socket with the same file descriptor. Now when the first thread comes out from timeout(in the native), it does not know that the stream has been closed an it passes the old fd number to system read(). As this fd is valid (because it got recreated), read succeeds.
So the native call SocketRead() in SocketInputStream class and the close() method should be synchronized blocks. While one thread is in native SocketRead(), we should not allow other threads to close the stream and set fd to null.
###@###.### 2003-07-28
|