kmatheussen / radium

A graphical music editor. A next generation tracker.
http://users.notam02.no/~kjetism/radium/
GNU General Public License v2.0
839 stars 36 forks source link

pure data: unnecessary listening after connection [PATCH linked] #523

Open titola opened 8 years ago

titola commented 8 years ago

It is a problem in pd (patch submitted).

Example with radium:

a) add 3 pd instruments;

   there are 3 connections with "wish" and useless listening
   on 3 ports (5400, 5401, 5402).

b) remove the created pd instruments;

   (radium continues to listen on 5400, 5401 and 5402)

c) add 3 pd instruments;

   there are 3 connections with "wish" and useless listening
   on 6 ports (5400, 5401, 5402, 5403, 5404, 5405).

d) remove the created pd instruments;

   (radium continues to listen on six ports)

e) goto [a] and add 6 ports as offset ;)

The patch for pd is here:

https://sourceforge.net/p/pure-data/patches/577/

and for radium:

--- radium/bin/packages/libpd-master/pure-data/src/s_inter.c~   2016-05-10 17:09:39.000000000 +0200
+++ radium/bin/packages/libpd-master/pure-data/src/s_inter.c    2016-05-10 17:12:40.000000000 +0200
@@ -1153,6 +1153,7 @@
 #if !defined(LIBPD)
             setuid(getuid());          /* lose setuid priveliges */
 #endif
+            sys_closesocket(xsock);    /* child doesn't listen */

 #ifndef __APPLE__
 // TODO this seems unneeded on any platform hans@eds.org
@@ -1260,6 +1261,7 @@
         {
             sys_set_priority(1);
             setuid(getuid());      /* lose setuid priveliges */
+            sys_closesocket(xsock);    /* child doesn't listen */
             if (pipe9[1] != 0)
             {
                 dup2(pipe9[0], 0);
@@ -1316,9 +1318,9 @@

         sys_guisock = accept(xsock, (struct sockaddr *) &server, 
             (socklen_t *)&len);
-#ifdef OOPS
+
         sys_closesocket(xsock);
-#endif
+
         if (sys_guisock < 0) sys_sockerror("accept");
         if (sys_verbose)
             fprintf(stderr, "... connected\n");
kmatheussen commented 8 years ago

It's probably correct. I'll wait til tomorrow before I apply. Just put it into the back of the head in the meanwhile.

kmatheussen commented 8 years ago

I don't feel too safe about this. I don't want to apply it to the libpds repository, in case it causes something, and I forget about it. I'll see what Miller says first. Perhaps you can make a pull request to the libpds repository instead? https://github.com/kmatheussen/libpd

titola commented 8 years ago

Don't worry, I think a patch for pd is enough.

The change is safe for me. Probably the soap opera with sys_startgui is the follow: the socket was closed after the connection but not for the children. It was a bug and they "fixed" it by using a #ifdef around sys_closesocket. But that's only a workaround. Under the hood the child tries to listen but it silently fails because the parent continues to listen on the same port. The patch is the correct fix where the socket is closed for all (parent and children). I don't see side effects because the correct behaviour of sys_startgui is

- listen for a connection
- accept and connect
- return (without other useless listening)

Besides, the listening after sys_startgui is without control because the socket "xsock" is local.