Jetty should rely on socket.shutdownOutput() to close sockets

I'm not so sure this is correct.

Jetty and other network servers worked perfectly well before the shutdown methods were added to the
API. close also flushes data in buffers and does an orderly shutdown.

Now a client may have an issue if it tries to write a request, close the output and read the response.
But a) jetty is mainly a server; b) close is not called by the client or the server until the response is
sent/received and complete; c) using close to signify the end of a request content is not legal HTTP.

From what I can see, the implementations of the shutdown methods are very problematic.
Many JVMs and socket types have them as not implemented.

So unless you can demonstrate an actual loss of data due to an early close, then I'm not going
to act on this issue. But I will continue to do a bit more research and any pointers you may have would
be appreciated.

From what I can see, the implementations of the shutdown methods are very problematic.
Many JVMs and socket types have them as not implemented.

Can you link to some research on that? I find that surprising.

I'm attaching a test case that fails when sockets are being closed. It approximates what Restlet is doing for a chunked request.

Test case that fails when SocketEndpoint.close() closes a client socket

Attaching a simplified version of the first test case.

The problem seems to be related to

• Chunked Encoding
• Connection: close
• Writing the "chunked" bytes one at a time

The exception I get is:

java.net.SocketException: Software caused connection abort: recv failed
	at java.net.SocketInputStream.socketRead0(Native Method)
	at java.net.SocketInputStream.read(SocketInputStream.java:129)
	at java.net.SocketInputStream.read(SocketInputStream.java:182)
	at org.mortbay.jetty.Jetty547TestCase2.read(Jetty547TestCase2.java:87)
	at org.mortbay.jetty.Jetty547TestCase2.test547(Jetty547TestCase2.java:60)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at junit.framework.TestCase.runTest(TestCase.java:164)
	at junit.framework.TestCase.runBare(TestCase.java:130)
	at junit.framework.TestResult$1.protect(TestResult.java:106)
	at junit.framework.TestResult.runProtected(TestResult.java:124)
	at junit.framework.TestResult.run(TestResult.java:109)
	at junit.framework.TestCase.run(TestCase.java:120)
	at junit.framework.TestSuite.runTest(TestSuite.java:230)
	at junit.framework.TestSuite.run(TestSuite.java:225)
	at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)

Kevin,

OK - you have me interested now!

However, I note that the client does in fact received all the data.
It is only on the read AFTER the last byte that the exception is thrown, when I thought -1 would
have been a more correct response.

So maybe the client needs to ignore reset responses if the connection:close header is set.

It is rather strange, as wireshark shows that most closes are done with an orderly TCP/IP FIN and ACK
only a few are actually terminated with a RST? shutdownOutput was added in java 1.3, so java 1.0 to 1.2 must have been able to do an orderly shutdown of sockets and I'm amazed they would break existing code by changing close().

Anyway, I will play with shutdown output in jetty 7 and see if it causes any problems

The SelectChannelEndpoint already called shutdownOutput - just before it called close!
I've modified SocketEndpoint to do the same, but for now I have left the close in.

This appears to prevent the reset being sent!

Can you test this solution. (note I think the client still needs to better handle RST).

cheers

Jetty547TestCase3 still fails on the trunk. SocketEndPoint.close() still calls socket.close() and ChannelEndPoint.close() still calls channel.close()

Kevin,

We have to call close, because shutdownOutputStream is not universally supported on all JVMs.
On some socket types/JVMs it is implemented as a noop and on others it throws an unsupportedOperationException.

I've discussed the issue with the IO guys at Sun, and they believe that close should always initiate an orderly shutdown. The fact that it is sometimes sending a RST appears to be a JVM bug. We can try calling shutdownOutput to attempt to avoid this JVM bug, but we can't avoid calling close to ensure that we really do close the connection.

I suggest that you open a bug against the JVM for the RST sent on close. It simply should not do that.

I'll have a look again at your test case, because when I last ran it, the shutdownOutput before the close made that test pass for me.

It was an exchange while we were discussing the NIO2 API. I asked if a call to close should initiate a normal shutdown of the connection or not.
The answer was that they believed that it should. When I said that sometimes we are observing RST, the response was simply along the lines that they didn't think that it should do that.

I'm currently in transit, but will run your test harness again towards the end of the week.

I think I hit this exact problem when using apache as reverse proxy in front of jetty. As java documents [*] state, socket.close() means that the application is done with the socket, both input and output. As on tcp-level, the only way for system to close both input and output streams of a tcp connection, is to send a RST packet. And this is what java implementations do.They first send FIN/ACK for the output stream, followed by RST to close the input stream as well.

This is not what apache mod_proxy expects. We have a case where client does a big http POST, say 2 megs. Apache proxy begins to transmit this request to the jetty. After reading a few kBs, jetty notices that the request needs to be authenticated. At this point jetty responds with HTTP 401, and closes the connection (socket.close() --> FIN + RST). Apache mod_proxy receives response and the FIN, immediately followed by RST-message.

Now, what should mod_proxy do? TCP specification says that after RST all communication should immediately stop. Mod_proxy still has lots of request data to send. Some data hes been received, but so has RST. At this point, mod_proxy decides that the connection is broken, and result code HTTP 502 is returned to the client.

I think this is a little gray area regarding the functionality of the HTTP protocol. Can server break the connection before the whole request is sent, and assume the client will handle the already-sent response? And of course it is pretty silly to send a 2 meg request twice, just to authenticate the user. All I know is, that currently this behaviour breaks proxied connections. If I could decide, I had this fixed in mod_proxy, as it would not break existing applications. However, jetty could provide an alternative (cleaner?) way of handling http-requests (consume all the request data before closing connection).

[*] http://java.sun.com/j2se/1.5.0/docs/guide/net/articles/connection_release.html

The more I think about this, the more I'm convinced that jetty behaves incorrectly. HTTP server cannot assume that the client reads the server response before the whole request has been sent. For example, apache mod_proxy sends the whole request before even trying to read the response. As jetty sends RST (socket.close()) before the whole request is read, the response is never even read.

I'll ponder this for a little while.... but in terms of sending a 2M request twice because of authentication, the client really should be using Expect:Continues to avoid sending the body until the server has had a chance to send a 401.

Ville,
Once a non persistent response has been written, we are currently calling shutdownOutput and then close.
It looks like it is pretty easy to delay the call to close until the request has been entirely read. I'm working on a patch for this and if it works it would be great if you could test this.

I will probably only do this in jetty-7, as jetty-6 is in maint mode. Is this a problem for you?

cheers

I've committed a fix to jetty-7 r1948
It would be really great if you could test ASAP as we have a release due.

Thanks for the quick reply! I agree, that the client should use Expect/Continues. The client is Apache Maven 2.2.1. Unfortunately I have no easy way to see the headers of these requests, as they are using https. They are HTTP1.1 requests. There seems to be at least some discussion about the problem regarding maven, but I can't confirm if maven can support use of continues.

In the middle, we have apache http, from RedHat EL5.5, version 2.2.3-43.el5. I think mod_proxy implements http1.1 compliant proxy, so it should support passing continues to the backend server. In our setup, httpd does ssl offloading, and proxies (reverse proxy) requests to the backend server. Many forums suggested forcing the proxy to use http1.0 for proxied requests as a solution to 50x errors, as well as disabling keepalives. Dumping backend server traffic however revealed this RST-behaviour, which causes 502 errors.

On the backend server side we have Nexus (nexus.sonatype.org), which comes bundled with jetty 6.1.12. As this is a bundled setup, and I don't have access to this server, it may prove difficult to try this out. At least when it comes to upgrading major numbers...

Well I'm not going to be able to fix a bundled 6.1.12 either.
But if you raise an issue with the sonatype folks and point them to this issue, then on there next release cycle, we might be able to get something into a jetty-6 maintenance release for them.

Fixed for jetty-6 now also. r6192.