Bug 15430 - Cannot interrupt blocking I/O calls with close()
Summary: Cannot interrupt blocking I/O calls with close()
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libgcj (show other bugs)
Version: 4.0.0
: P2 normal
Target Milestone: 4.1.0
Assignee: David Daney
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2004-05-14 01:23 UTC by Bryce McKinlay
Modified: 2005-11-25 19:10 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-07-12 22:00:09


Attachments
Test case (1.08 KB, text/plain)
2004-05-14 01:26 UTC, Bryce McKinlay
Details
Program that demonstrates how shutdown can solve the problem. (696 bytes, text/plain)
2005-11-09 17:46 UTC, David Daney
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Bryce McKinlay 2004-05-14 01:23:21 UTC
When close() is called on a Socket or ServerSocket, it should cause a
SocketException to be thrown on any thread which is blocked on I/O operations on
that socket.

In libgcj, no exception is thrown, and the I/O thread remains blocked even
though the socket has been closed.

The attached test case demonstrates the problem for accept() and read(). The
problem most likely occurs for other calls such as write() and connect() as
well. This test case should be extended to cover these other calls, and added to
mauve.
Comment 1 Bryce McKinlay 2004-05-14 01:26:39 UTC
Created attachment 6278 [details]
Test case
Comment 2 Andrew Pinski 2004-05-14 01:29:28 UTC
Confirmed.
Comment 3 Andrew Pinski 2004-08-14 06:59:00 UTC
FAIL: wrong exception: java.io.IOException: Bad file descriptor
FAIL: read() did not interrupt on close().
FAIL: read() did not interrupt on close().
FAIL: accept() did not interrupt on close().
Comment 4 Michael Koch 2005-01-15 22:30:01 UTC
I'm working on it 
Comment 5 David Daney 2005-11-09 17:46:31 UTC
Created attachment 10189 [details]
Program that demonstrates how shutdown can solve the problem.

Compile socktest.c thusly:

gcc -g -o soctest soctest.c -lpthread

Then run it and from a different window telnet localhost 4455

Do not type anything in the telnet window.

When the socket is shutdown the read returns with a value of zero.

I guess we should shutdown as well as close in the socket code.
Comment 6 Andrew Haley 2005-11-09 18:22:23 UTC
Two things:

Does this work for fds that aren't associated with sockets?

It doesn't quite avoid the need for locking, since we still need to make sure that we only close an fd once.
Comment 7 David Daney 2005-11-09 19:24:15 UTC
Here is the first version of my patch:

http://gcc.gnu.org/ml/java-patches/2005-q4/msg00176.html
Comment 8 David Daney 2005-11-10 00:34:18 UTC
New patch at:

http://gcc.gnu.org/ml/java-patches/2005-q4/msg00179.html
Comment 9 David Daney 2005-11-15 19:11:57 UTC
Subject: Bug 15430

Author: daney
Date: Tue Nov 15 19:11:53 2005
New Revision: 107036

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=107036
Log:
	PR libgcj/15430
	* gnu/java/net/natPlainSocketImplPosix.cc (throw_on_sock_closed): New
	function.
	(accept): Call it.
	(close): Call shutdown before closing.
	(read()): Call read_helper with proper parameters.
	(read(buffer, int, int)): Likewise.
	(read_helper):  Pass pointer to the PlainSocketImpl, remove native_fd
	and timeout parameters.  Make prototype to match. Use 
	pointer to PlainSocketImpl to access members. Call throw_on_sock_closed
	in two places.

Modified:
    trunk/libjava/ChangeLog
    trunk/libjava/gnu/java/net/natPlainSocketImplPosix.cc

Comment 10 David Daney 2005-11-15 19:16:31 UTC
Fixed by committed patch.
Comment 11 Guilhem Lavaux 2005-11-25 19:10:04 UTC
I feel a potential race condition in this patch. What happens in that case ?

thread 2:
  => PlainSocketImpl.accept
  => enters _Jv_select

thread 1:
  => shutdown socket
  => close socket
thread 3:
  => create another socket which occurs to have the same fd.
thread 2:
  => select wakes up
  => we call accept with the old fd

thread 1:
  => set native_fd to -1

I expect this scheme may be a flaw and it is not forbidden by the current code. A simple fix would be to add a synchronization lock before accepting the connection.

Wrong ?