master build broken [macOS]

master build broken [macOS]

[Filipp Gunbin]

  CCLD     emacsclient
emacsclient.c:1426:58: error: use of undeclared identifier 'SOCK_CLOEXEC'
      HSOCKET s = cloexec_socket (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
                                                         ^
1 error generated.

Looks like it's the recent commit:
* 5c412405c7..: Paul Eggert 2018-12-02 emacsclient: don’t leak socket to child processes

Thanks,
Filipp.

Re: master build broken [macOS]

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Date: Mon, 03 Dec 2018 14:47:31 +0300
> 
>   CCLD     emacsclient
> emacsclient.c:1426:58: error: use of undeclared identifier 'SOCK_CLOEXEC'
>       HSOCKET s = cloexec_socket (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
>                                                          ^
> 1 error generated.

Does the below fix the problem?

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index c430217..7de3665 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -1423,7 +1423,7 @@ set_local_socket (char const *server_name)
 
   if (sock_status == 0)
     {
-      HSOCKET s = cloexec_socket (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+      HSOCKET s = cloexec_socket (AF_UNIX, SOCK_STREAM, 0);
       if (s < 0)
 	{
 	  message (true, "%s: socket: %s\n", progname, strerror (errno));

Re: master build broken [macOS]

[Filipp Gunbin]

On 03/12/2018 14:33 +0200, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Date: Mon, 03 Dec 2018 14:47:31 +0300
>>
>>   CCLD     emacsclient
>> emacsclient.c:1426:58: error: use of undeclared identifier 'SOCK_CLOEXEC'
>>       HSOCKET s = cloexec_socket (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
>>                                                          ^
>> 1 error generated.
>
> Does the below fix the problem?
>
> diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
> index c430217..7de3665 100644
> --- a/lib-src/emacsclient.c
> +++ b/lib-src/emacsclient.c
> @@ -1423,7 +1423,7 @@ set_local_socket (char const *server_name)
>
>    if (sock_status == 0)
>      {
> -      HSOCKET s = cloexec_socket (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
> +      HSOCKET s = cloexec_socket (AF_UNIX, SOCK_STREAM, 0);
>        if (s < 0)
>  	{
>  	  message (true, "%s: socket: %s\n", progname, strerror (errno));

Yes, it does, thanks.

Re: master build broken [macOS]

[Paul Eggert]

Thanks for the fix; I installed it. GNU/Linux and Solaris <sys/socket.h> 
have SOCK_CLOEXEC and I didn't catch the typo for non-SOCK_CLOEXEC 
systems like macOS.

It is odd that even after all these years there's no way to create 
sockets in a thread-safe way on macOS. This doesn't matter for 
emacsclient but I assume it does matter for src/process.c.

Re: master build broken [macOS]

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 3 Dec 2018 08:27:20 -0800
> Cc: Filipp Gunbin <fgunbin@fastmail.fm>, emacs-devel@gnu.org
> 
> Thanks for the fix; I installed it. GNU/Linux and Solaris <sys/socket.h> 
> have SOCK_CLOEXEC and I didn't catch the typo for non-SOCK_CLOEXEC 
> systems like macOS.
> 
> It is odd that even after all these years there's no way to create 
> sockets in a thread-safe way on macOS. This doesn't matter for 
> emacsclient but I assume it does matter for src/process.c.

Btw, why do we want to mark the emacsclient sockets non-inheritable?
According to my reading of the code, we set up the socket after we
fork/exec, so socket inheritance to the Emacs daemon is not the
problem, right?  If so, what inheritance would be the problem?  Just
wondering.

Re: master build broken [macOS]

[Paul Eggert]

On 12/3/18 9:57 AM, Eli Zaretskii wrote:
>   what inheritance would be the problem

The exec of the alternate editor inside the 'fail' function. Hmm, now I 
see there's no fork there, so I was mistaken in calling it a child 
process; it's still the emacsclient process (though of course the 
alternate editor could itself create children).