Re: Review of gnio, round 1

[Mark Mielke <mark mark mielke cc>]

Dan Winship wrote:

Alexander Larsson wrote:
  

On Mon, 2009-04-27 at 13:22 -0400, Dan Winship wrote:
    

g_socket_finalize closes socket even if g_socket_close() is already
closed, need to add an is_closed variable. We should also check this in
all operations so that we don't accidentally call stuff on an fd that
may have been reused by now.
        

libsoup's solution here is to implement "close" as
shutdown(fd,SHUT_RDWR), and only actually call close(fd) at finalization
time.
      

Whats the advantage here over close() and then setting some is_closed
bit?
    

Well, you don't need an is_closed bit, and you don't need to add a
special check to every op. You just try to recv() or send() or whatever,
and get back ENOTCONN (or whatever it is), and handle that normally just
like if it had been the remote end that closed the connection.
  

Leaving a file descriptor open has a cost. Calling I/O operations on a closed handle should give an E_PROGRAMMER_ERROR exception - not a system call errno. :-)

In libsoup it's also important because it's thread-safe/non-racy. That
may not be a relevant criterion for GSocket, although the source
returned by g_socket_create_source() could create similar problems; you
need to be certain that any such sources are destroyed before you call
g_socket_close(), or else they could trigger falsely if the fd gets
reused. Delaying close() until finalization makes that easy because then
you can just have the source hold a ref on the socket to ensure that the
fd doesn't get closed.
  

It's a clever solution to the thread-safe issue - but it doesn't seem like a great solution. Leaving file descriptors open "in case" they happen to still have references floating around in memory that might be used, and waiting until the object is removed or program termination before closing the file descriptor is a bit hacky / non-scaleable. I'm sure it works great if the program only uses one or two file descriptors.

Still, I can't think of a better solution for dealing with inherently buggy code that might accidentally use a file descriptor from another thread after it has been closed but before is_closed is set without locking the system down with mutexes which would become painful.

So, it's either use up file descriptors in case of a broken caller, or don't use up file descriptors, but require the caller to be written sensibly.

Cheers,
mark

-- 
Mark Mielke <mark mielke cc>