send should check socket status and fail if the socket is closed

[Yuras]

The Network.Socket.close claims that

All future operations on the socket object will fail

It changes socket state to Closed and calls close(2) C function, so OS is free to reuse the FD.

But Network.Socket.send and friends don't check socket status. When sending data to a closed socket usually an exception is thrown (close(2) returns EBADF), but there is a chance that the FD is reused after close but before send, so the data will be silently sent to a wrong destination.

One can argue that writing to a closed socket is a bad practice anyway, then please consider it as a documentation bug.

[Yuras]

Bump.

Will you accept a PR fixing the code to match the documentations? Note that is will add one withMVar per send and recv, also it will serialize sending and receiving.

Alternatively will you accept a PR fixing the documentation to match the implementation?

[eborden]

Please submit the PR and we can discuss the cost and benefit. Checking seems like the right thing to do.

[Yuras]

@eborden Thank you for your response!

Which PR should I send? The one that fixes the code or that fixes documentation?

I don't think withMVar really makes sense here. @Peaker suggested read/write lock, but it is out of my abilities because it is too intrusive and I can't test all CPP configurations.

Checking socket status without locking can help to make the issue more visible, though it will not really fix. I think that fixing documentation is the first step.

[eborden]

I'd prefer the bug fix over a doc fix. A naive implementation using an mvar can get the ball rolling and we can refine from there. Likely the read/write lock is the final destination, but high level code can suss out some details first. Thanks for being persistent with this issue.

[kazu-yamamoto]

I discussed this with my friend.

• This bug is serious and should be fixed somehow.
• I don't want to modify the current API. The pull request introduces significant overhead. I would avoid this way.
• So, let's create Safe module. send and other APIs in Safe module checks the socket status.

What do you think, guys?

[Yuras]

It is definitely better then doing nothing. And the documentation for the old API should be updated to describe the issue and point to Safe module.

[enolan]

I'm working on a PR with a Network.Socket.Safe module, along with Network.Socket.ByteString.Safe and Network.Socket.ByteString.Lazy.Safe.

[kazu-yamamoto]

I close this in favor of #212.