[network] #6: Network.Socket.sClose should change socket status

[network]

#6: Network.Socket.sClose should change socket status
---------------------+------------------------------------------------------
 Reporter:  tibbe    |       Owner:     
     Type:  defect   |      Status:  new
 Priority:  major    |   Milestone:     
Component:  network  |     Version:     
 Keywords:           |  
---------------------+------------------------------------------------------
 Initially reported here: http://hackage.haskell.org/trac/ghc/ticket/2996

 ----

 The current implementation of sClose in Network.Socket leaves the socket
 in a Connected state but frees the underlying OS resource. Thus, any
 further operations on the socket may apply to a different OS object if
 something else was assigned the same FD after the socket was closed. For
 example, sClose'ing the socket a second time can lead to some arbitrary
 other file or socket being closed.

 The attached test case was run in ghci 6.8.2 on Debian Linux sid, though
 by inspection of the Network.Socket source in 6.10.1, this is still
 present and the test case should work. It takes advantage of the way *NIX
 allocates file descriptors to demonstrate specific misbehaviors, so may or
 may not misbehave in the intended way on other platforms.

 ----

 Test case

 ----

 Simple patch to add a Closed state and make sClose idempotent. Operations
 that perform state transitions already check the socket state and will
 refuse to operate on a Closed socket, but this does not add checks for
 other operations like send/recv.

 ----

 I should mention that closed-state.patch is against the 6.8.2 network
 library. It applies cleanly (with offsets) against the 6.10.1 library, but
 I can't test it.

 ----

 Proposed patch in addition to closed-state.patch that adds checks for a
 valid FD in all operations. This is a fairly simple approach. In
 particular, it punts on the problem of one thread closing the socket in
 the middle of a socket operation in another thread. Arguably, the
 semantics of this are sufficiently unclear (especially for blocking
 operations) that applications should be doing their own synchronization in
 such situations.

 ----

 Thanks for the patches! We'll take a look.

 ----

 tibbe, this looks related to things you've been working on recently, and I
 think you are considering becoming the network maintainer? Can you take a
 look please?

 Thanks!

 ----

 Sorry for the late reply. closed-state.patch looks fine to me. As for the
 second patch I think we need to figure out the performance consequences of
 checking an MVar for every op before we make the change.

 Perhaps it would be worthwhile to write down which guarantees the API
 currently gives and could give when used in multi-threaded code. I'm not
 even entirely sure what guarantees we give at the moment, without your
 patches. If nothing else that would serve as good documentation.

 ----

 I pushed the closed-state.patch.

-- 
Ticket URL: <http://trac.haskell.org/network/ticket/6>
network <http://projects.haskell.org/network/>
Networking-related facilities