http://d.puremagic.com/issues/show_bug.cgi?id=1491

           Summary: if working with timed-out socket, SIGPIPE will kill
                    program
           Product: D
           Version: unspecified
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Keywords: patch
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: default_357-line@yahoo.de


On linux, when trying to work with a socket that has already expired, the
signal SIGPIPE is raised.
If unchecked, it will terminate the program.
Since there is a defined behavior for trying to work with timed-out sockets
(send/receive return -1), we can safely ignore this signal.
The following patch for gdc's std/thread.d (should be equally applicable to
dmd) implements this:

diff socket.d.old socket.d -u
--- socket.d.old        2007-09-11 03:05:54.000000000 +0200
+++ socket.d    2007-09-11 03:10:30.000000000 +0200
@@ -126,6 +126,17 @@
        }
 }

+version (Win32) { }
+else
+{
+       extern(C)
+       {
+               typedef void function(int) sighandler_t;
+               sighandler_t signal(int signum, sighandler_t handler);
+               const int SIGPIPE=13;
+               const sighandler_t SIG_IGN=cast(sighandler_t)cast(void*)1;
+       }
+}

 static this()
 {
@@ -140,6 +151,10 @@
                if(val) // Request Winsock 2.2 for IPv6.
                        throw new SocketException("Unable to initialize socket
library", val);
        }
+       else
+       {
+               signal(SIGPIPE, SIG_IGN);
+       }
 }


-- 

d-bugmail@puremagic.com wrote:

> On linux, when trying to work with a socket that has already expired, the
> signal SIGPIPE is raised.
> If unchecked, it will terminate the program.
> Since there is a defined behavior for trying to work with timed-out
> sockets (send/receive return -1), we can safely ignore this signal.
> The following patch for gdc's std/thread.d (should be equally applicable
> to dmd) implements this:

No! NACK!

Signals are a global state.

Libraries should never modify them in that manner, since the defaults are well documented in signal(7) and programs might expect this behaviour.

The default setting for SIGPIPE is to terminate the program, which is correct for simple programs not handling signals at all.

The preferred solution in Linux is to provide the flag MSG_NOSIGNAL in the flags parameter of send(2), sendto(2) and sendmsg(2).

But this symbol must be generated by gen_unix.c first, if defined.
(in function c_socket())


Best Regards

Ingo Oeser

Ingo Oeser wrote:
> d-bugmail@puremagic.com wrote:
> 
>> On linux, when trying to work with a socket that has already expired, the
>> signal SIGPIPE is raised.
>> If unchecked, it will terminate the program.
>> Since there is a defined behavior for trying to work with timed-out
>> sockets (send/receive return -1), we can safely ignore this signal.
>> The following patch for gdc's std/thread.d (should be equally applicable
>> to dmd) implements this:
> 
> No! NACK!
> 
> Signals are a global state.
> 
> Libraries should never modify them in that manner, since the defaults are well documented in signal(7) and programs might expect this behaviour.
I disagree on this. The only behavior std/socket.d should exhibit is the behavior documented in http://digitalmars.com/d/phobos/std_socket.html. Unix signals are a platform specific feature that somebody writing a platform independent program should _not_ have to mess with.

> 
> The default setting for SIGPIPE is to terminate the program, which is correct for simple programs not handling signals at all.
> 
But this behavior is neither documented in std_socket.html, nor is it platform independent, nor is there a platform independent way to _prevent it_.

> The preferred solution in Linux is to provide the flag MSG_NOSIGNAL in the flags parameter of send(2), sendto(2) and sendmsg(2).
> 
Okay, well great! If there's a proper way to suppress this behavior, by all means, let's do that instead! But please, don't force the user to write platform dependent code. That's what standard libraries are for.

> But this symbol must be generated by gen_unix.c first, if defined.
> (in function c_socket())
> 
> 
Right, my patch isn't very clean. It was mainly along the lines of "hey, this worked for me" :)
> Best Regards
> 
> Ingo Oeser
 --downs

Alright, here's a revised version of the patch.

diff config/gen_unix.c.old config/gen_unix.c -u; diff -u
std/socket.d.old std/socket.d
- --- config/gen_unix.c.old       2007-09-12 06:33:55.000000000 +0200
+++ config/gen_unix.c   2007-09-12 06:29:01.000000000 +0200
@@ -667,6 +667,7 @@
     CES( MSG_OOB );
     CES( MSG_PEEK );
     CES( MSG_DONTROUTE );
+    CES( MSG_NOSIGNAL );
     printf("}\n");
     printf("\n");
 }
- --- std/socket.d.old    2007-09-11 03:05:54.000000000 +0200
+++ std/socket.d        2007-09-12 06:37:08.000000000 +0200
@@ -1335,6 +1335,11 @@
        //returns number of bytes actually sent, or -1 on error
        int send(void[] buf, SocketFlags flags)
        {
+               version(Windows) { }
+               else
+               {
+                       flags |= MSG_NOSIGNAL;
+               }
                int sent = .send(sock, buf.ptr, buf.length, cast(int)flags);
                return sent;
        }

Downs wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Alright, here's a revised version of the patch.

That one was much better. Could you please fix the sendTo()
method calling .sendto(), too?

Many thanks!


Best Regards

Ingo Oeser

Downs wrote:

> Ingo Oeser wrote:
>> Signals are a global state.
>> 
>> Libraries should never modify them in that manner, since the defaults are well documented in signal(7) and programs might expect this behaviour.
> Unix signals are a platform specific feature that somebody writing a platform independent program should _not_ have to mess with.

Then simply don't touch them in a manner visible to the user.
Doing that is as useful as keeping locks open, which is also
not documented. Principle of least surprise should be applied here.

If you write a Unix daemon (sth. like a service in Windows),
you need to handle signals in a defined way. There is no way around
that, since your desired behaviour depends on your needs.

Face it: Windows and Unix are so different, that anything more complex
        than "hello world" needs to handle the subtle differences anyway.

>> The default setting for SIGPIPE is to terminate the program, which is correct for simple programs not handling signals at all.
>> 
> But this behavior is neither documented in std_socket.html, nor is it platform independent,

Interaction of between phobos and Unix systems is not documented in any *.html at all.

This leads to hard to find bugs, since many API details are only documented in the man pages on Unix systems and the API is implemented in the matching C libraries (e.g. the glibc on Linux). I'm happy, that I have the sources of the standard library to be able to find such side effects :-)

> nor is there a platform independent way to _prevent it_.

Correct! By googling around a bit I found another way to do this,
if MSG_NOSIGNAL is not available: setsockopt() with SO_NOSIGPIPE.
This might prevent the same problem on MacOS X. But I don't have the docs
for that one.

If everything else fails, one can ignore SIGPIPE before send() and sendTo()
and restore the old signal behaviour after the call. That is inefficient,
but at least correct and as transparent as possible.

> Okay, well great! If there's a proper way to suppress this behavior, by all means, let's do that instead! But please, don't force the user to write platform dependent code. That's what standard libraries are for.

Of course not! Yes, but the standard library is also the one, which has to be compatible in program behaviour with any other library.

How useful is a standard library, which doesn't work with millions of lines of code written already?

I like revolutionary ideas, but the should integrate in practise.


Best Regards

Ingo Oeser

Ingo Oeser wrote:
> Downs wrote:
> 
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Alright, here's a revised version of the patch.
> 
> That one was much better. Could you please fix the sendTo()
> method calling .sendto(), too?
> 
> Many thanks!
> 
> 
> Best Regards
> 
> Ingo Oeser

Sorry, I forgot. Sec ..

diff config/gen_unix.c.old config/gen_unix.c -u; diff -u
std/socket.d.old std/socket.d
- --- config/gen_unix.c.old       2007-09-12 06:33:55.000000000 +0200
+++ config/gen_unix.c   2007-09-12 06:29:01.000000000 +0200
@@ -667,6 +667,7 @@
     CES( MSG_OOB );
     CES( MSG_PEEK );
     CES( MSG_DONTROUTE );
+    CES( MSG_NOSIGNAL );
     printf("}\n");
     printf("\n");
 }
- --- std/socket.d.old    2007-09-11 03:05:54.000000000 +0200
+++ std/socket.d        2007-09-13 03:10:54.000000000 +0200
@@ -1335,6 +1335,11 @@
        //returns number of bytes actually sent, or -1 on error
        int send(void[] buf, SocketFlags flags)
        {
+               version(Windows) { }
+               else
+               {
+                       flags |= MSG_NOSIGNAL;
+               }
                int sent = .send(sock, buf.ptr, buf.length, cast(int)flags);
                return sent;
        }
@@ -1350,6 +1355,11 @@
         */
        int sendTo(void[] buf, SocketFlags flags, Address to)
        {
+               version(Windows) { }
+               else
+               {
+                       flags |= MSG_NOSIGNAL;
+               }
                int sent = .sendto(sock, buf.ptr, buf.length,
cast(int)flags, to.name(), to.nameLen());
                return sent;
        }
@@ -1365,6 +1375,11 @@
        /// ditto
        int sendTo(void[] buf, SocketFlags flags)
        {
+               version(Windows) { }
+               else
+               {
+                       flags |= MSG_NOSIGNAL;
+               }
                int sent = .sendto(sock, buf.ptr, buf.length,
cast(int)flags, null, 0);
                return sent;
        }

Excuse the copypaste, but I wasn't sure whether it was worth it making a
new private function for this.
 --downs

http://d.puremagic.com/issues/show_bug.cgi?id=1491


bugzilla@digitalmars.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




------- Comment #4 from bugzilla@digitalmars.com  2007-11-03 21:48 -------
Fixed dmd 1.023


--