sionescu / iolib

Common Lisp I/O library
http://common-lisp.net/project/iolib/
MIT License
141 stars 31 forks source link

(close) after (sockets:shutdown) causes error #1

Closed hraban closed 13 years ago

hraban commented 13 years ago

Hi,

Consider the following program, test-sockets.lisp:


(load (merge-pathnames "quicklisp/setup.lisp" (user-homedir-pathname)))

(ql:quickload "iolib.sockets")

(defun send-msg (msg &key shutdown)
  (let ((s (make-socket)))
    (format s "~A~%" msg)
    (when shutdown
      (sockets:shutdown s :read T :write T))
    (close s)))

(defun make-socket ()
  (sockets:connect
    (sockets:make-socket :address-family :internet)
    (sockets:ensure-address
      (sockets:ensure-hostname "localhost")
      :family :internet)
    :port 12345))

(send-msg "Test 1." :shutdown NIL)
(send-msg "Test 2." :shutdown T)

Starting nc -k -l 0.0.0.0 12345 on the same computer to accept the incoming connections, I observe the following:

Clisp

$ clisp test-sockets.lisp

There is no output besides the usual quicklisp loading messages. Netcat outputs "Test 1." and the return code of the command is 141.

SBCL

$ sbcl --script test-sockets.lisp

Exit code 1. The output consists of some standard compilation messages and an interesting message about the crash, + traceback: http://paste.kde.org/105811/. Again netcat only shows Test 1.

To my understanding, this is a problem. Clisp crashing without so much as an error message is definitely a problem (if done in the REPL that also crashes without error). SBCL stays alive, but still does not like the (close s) after shutdown. Is this not the intended use of shutdown?

Greetings,

Hraban

sionescu commented 13 years ago

That code is incorrect in that it doesn't flush the socket before shutdown. In the second test case, close after shutdown will try to flush the socket, which will signal an error.

The proper way to write the test code is this:

(defun send-msg (msg &key shutdown)
  (with-open-socket (s :address-family :internet :type :stream
                       :remote-host "localhost" :remote-port 12345)
    (format s "~a~%" msg)
    (finish-output s)
    (when shutdown
      (sockets:shutdown s :read t :write t))))
hraban commented 13 years ago

Ah, I see. Thanks for your answer.

This is slightly confusing to somebody coming from C where the exact opposite holds: you can not explicitly flush network sockets. You actually use shutdown to do so.

Would that perhaps warrant a note in the shutdown documentation? "Remember to flush before shutting down"?

Additionally, why is the socket not automatically flushed on shutdown?

sionescu commented 13 years ago

When using only syscalls, shutdown flushes the kernel buffer, but with iolib(just as with C's stdio) there is also the userland buffer which needs flushing.

Exercise: in C try socket -> fdopen -> fwrite -> shutdown -> fclose and see what happens. If you're lucky your libc's stdio stream is line-buffered and fwrite implicitly flushes, otherwise if it's fully buffered you'll get the same kind of error

If you want to avoid iolib buffering, use sockets:send-to - but you mustn't use cl:format or any other stream function on the socket, after all you can't call fprintf on a file descriptor, can you ?

hraban commented 13 years ago

Ah, yes, the userland buffers that are out of iolib's control. Good point. I thought the data was being buffered by the socket (Nagle), but of course there is also the lisp runtime itself.

However, I am still not entirely convinced that it would be a bad idea to have (iolib.sockets:shutdown) call (cl:finish-output) on the socket explicitly. Is there a strong reason not to do this?

What I always assumed to be the case in C: the buffers are associated with the fdopen() stream, not with the socket() file descriptor. The file descriptor is not linked to the stream (but the other way around) so a call to shutdown with that fd can not easily get the userland buffers associated with all streams that might be derived from this fd to flush them. When it can, however, it does first explicitly flush the buffers: fclose does that, for example.

With iolib, the stream buffers are directly associated with the socket (since they are the same object) and flushing them before shutdown is, unless I am mistaking, trivial. It sounds sensible to me that a shutdown on a socket would guarantee all write operations on its stream are honoured.

What do you think about this?

sionescu commented 13 years ago

One can legitimately want to shutdown a connection without flushing the buffers, so the two actions are better left separate