Skip to content

Conversation

dnadlinger
Copy link
Contributor

Previously, std.socket code assumed that MSG_NOSIGNAL existed on all platforms, which is not the case – std.c.osx.socket actually has a comment saying »Not defined in OS X, but we'll use them anyway«. This led to Socket.send() raising SIGPIPE on OS X if the peer had already closed the connection.

With this commit, std.socket sets the SO_NOSIGPIPE socket option on systems that support it for the same effect as MSG_NOSIGNAL on Linux.

Strictly speaking, removing the NOSIGNAL SocketFlag is a breaking API change, but I do not think there is any code actually using it, as it has always been set implicitly anyway.

@dnadlinger
Copy link
Contributor Author

Requires druntime pull request 36 for defining SO_NOSIGPIPE.

@andralex
Copy link
Member

andralex commented Jul 5, 2011

Thanks. David, could you please also solve the conflict preventing this from automatically merging? Thanks!

Previously, std.socket code assumed that MSG_NOSIGNAL existed on all platforms, which is not the case – std.c.osx.socket actually has a comment saying »Not defined in OS X, but we'll use them anyway«. This lead to Socket.send() raising SIGPIPE on OS X if the peer had already closed the connection.

With this commit, std.socket sets the SO_NOSIGPIPE socket option on systems that support it for the same effect as MSG_NOSIGNAL on Linux.

Strictly speaking, removing the NOSIGNAL SocketFlag is a breaking API change, but I do not think there is any code actually using it, as it has always been set implicitly by std.socket anyway.
@dnadlinger
Copy link
Contributor Author

Done – I wonder why GitHub doesn't display something like the Fork Queue for non-comitters…

andralex added a commit that referenced this pull request Jul 7, 2011
Disable SIGPIPE on send()ing to a Socket on OS X.
@andralex andralex merged commit 4b48fa2 into dlang:master Jul 7, 2011
kuettler pushed a commit to kuettler/phobos that referenced this pull request Feb 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants