error in server-running-p on M$

error in server-running-p on M$

[dhruva]

Hi,
 I get the following error when I run emacs built with MSVC on WXP
(M$) and execute 'server-running-p' function.

;; in .emacs
(require 'server)
(server-running-p)


Debugger entered--Lisp error: (error "Unknown address family")
  make-network-process(:name "server-client-test" :family local
:server nil :noquery t :service "e:/users/dhruva/server")
  byte-code("            \b	\"&\n! ˇ" [name server-socket-dir
delete-process make-network-process :name "server-client-test" :family
local :server nil :noquery t :service expand-file-name] 14)
  server-running-p()
  eval((server-running-p))
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp nil nil)

-dhruva

-- 
Contents reflect my personal views only!

Re: error in server-running-p on M$

[Juanma Barranquero]

2008/11/22 dhruva <dhruvakm@gmail.com>:

>  I get the following error when I run emacs built with MSVC on WXP
> (M$) and execute 'server-running-p' function.

Yes. `server-running-p' does not work for TCP sockets.

You can read some discussion at this thread:

http://lists.gnu.org/archive/html/emacs-devel/2008-11/msg00106.html

  Juanma

Re: error in server-running-p on M$

[Eli Zaretskii]

> Date: Sat, 22 Nov 2008 12:58:09 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> 
> 2008/11/22 dhruva <dhruvakm@gmail.com>:
> 
> >  I get the following error when I run emacs built with MSVC on WXP
> > (M$) and execute 'server-running-p' function.
> 
> Yes. `server-running-p' does not work for TCP sockets.
> 
> You can read some discussion at this thread:
> 
> http://lists.gnu.org/archive/html/emacs-devel/2008-11/msg00106.html

I don't see any practical conclusions reached in that thread.  Am I
missing something?

How about if we fix server-running-p now, to DTRT for TCP sockets?

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sat, Nov 22, 2008 at 13:51, Eli Zaretskii <eliz@gnu.org> wrote:

> I don't see any practical conclusions reached in that thread.  Am I
> missing something?

No. As it happens in many other discussions, there was no practical
conclusion reached.

> How about if we fix server-running-p now, to DTRT for TCP sockets?

Someone would have to answer my question, which is: what is the stated
purpose of `server-running-p'?

The changes I proposed would allow simple detection of running TCP
servers on the same machine (without using net services, which could
cause delays), as long as they are using the same configuration
(specifically, server-name). Do we want to do something about

  emacs --eval "(progn (setq server-name \"one\") (server-mode 1))"
  emacs --eval "(progn (setq server-name \"two\") (server-mode 1))"

?

  Juanma

Re: error in server-running-p on M$

[Eli Zaretskii]

> Date: Sat, 22 Nov 2008 14:00:16 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: dhruvakm@gmail.com, emacs-devel@gnu.org
> 
> On Sat, Nov 22, 2008 at 13:51, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > I don't see any practical conclusions reached in that thread.  Am I
> > missing something?
> 
> No. As it happens in many other discussions, there was no practical
> conclusion reached.

Perhaps we should use the bug tracker for tracking issues as well, not
just bug reports.

> > How about if we fix server-running-p now, to DTRT for TCP sockets?
> 
> Someone would have to answer my question, which is: what is the stated
> purpose of `server-running-p'?

In that thread, Stefan answered it.

> The changes I proposed would allow simple detection of running TCP
> servers on the same machine (without using net services, which could
> cause delays), as long as they are using the same configuration
> (specifically, server-name).

Can you point me to these changes, or post them?  I don't think I saw
any diffs in that thread.

> Do we want to do something about
> 
>   emacs --eval "(progn (setq server-name \"one\") (server-mode 1))"
>   emacs --eval "(progn (setq server-name \"two\") (server-mode 1))"
> 
> ?

Sorry, I don't follow: how is this related to the issue at hand?

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sat, Nov 22, 2008 at 14:20, Eli Zaretskii <eliz@gnu.org> wrote:

> Perhaps we should use the bug tracker for tracking issues as well, not
> just bug reports.

I think so.

> In that thread, Stefan answered it.

He said: "Currently it's not used (except in some people's .emacs).
The intended use is in start-server to try and prevent hijacking the
socket of a pre-existing server."  But that is just part of the
answer, I think.

> Can you point me to these changes, or post them?  I don't think I saw
> any diffs in that thread.

I didn't send any diffs, just posted a little snippet of code, because
the change is almost trivial. See the attached patch, which
illustrates the problem I'm talking about.

> Sorry, I don't follow: how is this related to the issue at hand?

Because, if the only intent of `server-running-p' is to "prevent
hijacking the socket" of another server, the name is quite misleading.
It does not detect (even now, for local sockets) whether another
instance is running; it only detects whether another instance is
running _with_the_same_server_setup_ (presumably because the intention
is not disturbing it).

That's why I ask about two consecutive instances of Emacs using
different names, or switching from local to TCP sockets.
`server-running-p' won't detect the first one. If that is the intended
effect, then the function should be called
`is-server-socket-available-p'.

             Juanma


Index: lisp/server.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/server.el,v
retrieving revision 1.173
diff -u -2 -r1.173 server.el
--- lisp/server.el	5 Nov 2008 21:16:17 -0000	1.173
+++ lisp/server.el	7 Nov 2008 15:37:36 -0000
@@ -537,5 +537,15 @@
   (unless name (setq name server-name))
   (condition-case nil
-      (progn
+      (if server-use-tcp
+	  (with-temp-buffer
+	    (insert-file-contents-literally
+	     (expand-file-name server-name server-auth-dir))
+	    (and (looking-at "127\.0\.0\.1:[0-9]+ \\([0-9]+\\)")
+		 (let ((case-fold-search t)
+		       (proc (assq 'comm
+				   (system-process-attributes
+				    (string-to-number (match-string 1))))))
+		   (and proc
+			(string-match-p "emacs" (cdr proc))))))
 	(delete-process
 	 (make-network-process

Re: error in server-running-p on M$

[Eli Zaretskii]

> Date: Sat, 22 Nov 2008 14:37:02 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: emacs-devel@gnu.org
> 
> He said: "Currently it's not used (except in some people's .emacs).
> The intended use is in start-server to try and prevent hijacking the
> socket of a pre-existing server."  But that is just part of the
> answer, I think.

I'm okay with going with this partial answer.

> Index: lisp/server.el
> ===================================================================
> RCS file: /sources/emacs/emacs/lisp/server.el,v
> retrieving revision 1.173
> diff -u -2 -r1.173 server.el
> --- lisp/server.el	5 Nov 2008 21:16:17 -0000	1.173
> +++ lisp/server.el	7 Nov 2008 15:37:36 -0000
> @@ -537,5 +537,15 @@
>    (unless name (setq name server-name))
>    (condition-case nil
> -      (progn
> +      (if server-use-tcp
> +	  (with-temp-buffer
> +	    (insert-file-contents-literally
> +	     (expand-file-name server-name server-auth-dir))
> +	    (and (looking-at "127\.0\.0\.1:[0-9]+ \\([0-9]+\\)")
> +		 (let ((case-fold-search t)
> +		       (proc (assq 'comm
> +				   (system-process-attributes
> +				    (string-to-number (match-string 1))))))
> +		   (and proc
> +			(string-match-p "emacs" (cdr proc))))))
>  	(delete-process
>  	 (make-network-process
> 

Does anyone see any problems with this change?  If not, I suggest to
install it.

Re: error in server-running-p on M$

[Ulrich Mueller]

>>>>> On Sat, 22 Nov 2008, Eli Zaretskii wrote:

>> Index: lisp/server.el
>> ===================================================================
>> RCS file: /sources/emacs/emacs/lisp/server.el,v
>> retrieving revision 1.173
>> diff -u -2 -r1.173 server.el
>> --- lisp/server.el	5 Nov 2008 21:16:17 -0000	1.173
>> +++ lisp/server.el	7 Nov 2008 15:37:36 -0000
>> @@ -537,5 +537,15 @@
>> (unless name (setq name server-name))
>> (condition-case nil
>> -      (progn
>> +      (if server-use-tcp
>> +	  (with-temp-buffer
>> +	    (insert-file-contents-literally
>> +	     (expand-file-name server-name server-auth-dir))
>> +	    (and (looking-at "127\.0\.0\.1:[0-9]+ \\([0-9]+\\)")
>> +		 (let ((case-fold-search t)
>> +		       (proc (assq 'comm
>> +				   (system-process-attributes
>> +				    (string-to-number (match-string 1))))))
>> +		   (and proc
>> +			(string-match-p "emacs" (cdr proc))))))
>> (delete-process
>> (make-network-process
>> 

> Does anyone see any problems with this change?  If not, I suggest to
> install it.

Shouldn't it try to actually connect to the socket (in the
server-use-tcp case), to find out if the server is functional?
That would also be consistent with the behaviour for local sockets.

Ulrich

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sat, Nov 22, 2008 at 15:13, Ulrich Mueller <ulm@kph.uni-mainz.de> wrote:

> Shouldn't it try to actually connect to the socket (in the
> server-use-tcp case), to find out if the server is functional?
> That would also be consistent with the behaviour for local sockets.

The idea is to run a simple test, that does not have the risk of
incurring in a timeout.

Also, if the server authentication file says that Emacs process PID is
using port P, and there's really an Emacs of pid PID but it is no
longer answering to requests on port P, that is PID's problem, not the
current instance's.

  Juanma

Re: error in server-running-p on M$

[Ulrich Mueller]

>>>>> On Sat, 22 Nov 2008, Juanma Barranquero wrote:

>> Shouldn't it try to actually connect to the socket (in the
>> server-use-tcp case), to find out if the server is functional?
>> That would also be consistent with the behaviour for local sockets.

> The idea is to run a simple test, that does not have the risk of
> incurring in a timeout.

This doesn't catch all cases. For example, the server process could be
running on a remote machine, with the port tunnelled over ssh.

> Also, if the server authentication file says that Emacs process PID is
> using port P, and there's really an Emacs of pid PID but it is no
> longer answering to requests on port P, that is PID's problem, not the
> current instance's.

I think that the false positives are not the problem, but the false
negatives (i.e. port is in use, but server-running-p returns nil) are.

Ulrich

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sat, Nov 22, 2008 at 15:29, Ulrich Mueller <ulm@kph.uni-mainz.de> wrote:

> This doesn't catch all cases.

Of course it doesn't.

> I think that the false positives are not the problem, but the false
> negatives (i.e. port is in use, but server-running-p returns nil) are.

Ports are assigned at random. The new instance is not going to reuse
the port in the authentication file; we're starting the server with
:server t.

  Juanma

Re: error in server-running-p on M$

[Ulrich Mueller]

>>>>> On Sat, 22 Nov 2008, Juanma Barranquero wrote:

>> I think that the false positives are not the problem, but the false
>> negatives (i.e. port is in use, but server-running-p returns nil)
>> are.

> Ports are assigned at random. The new instance is not going to reuse
> the port in the authentication file; we're starting the server with
> :server t.

As an example: The auth file is on an NFS mounted filesystem, and the
Emacs server is running on the machine of the NFS server. How would
assigning a random port number help here?

With the proposed test, server-running-p will return nil in this case,
since it will find an auth file but no associated Emacs process.

Ulrich

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sat, Nov 22, 2008 at 16:35, Ulrich Mueller <ulm@kph.uni-mainz.de> wrote:

> As an example: The auth file is on an NFS mounted filesystem, and the
> Emacs server is running on the machine of the NFS server.

You're approaching my original question: what is the intended use of
`server-running-p', and how do we want it to work when different
instances of Emacs use different server setups. Of course there is no
problem in having several servers, even on the same machine. The
problem is one hiding the other.

For your case to be a problem, you must be starting a server in the
local machine set up such its authentication file clobbers the one on
the NFS server. That is one instance of the problem I discussed, but
you don't need such a complicate setup. Just start a local TCP server,
and then another with the same server name. Lo, you just rendered the
first one inaccessible.

> How would assigning a random port number help here?

> With the proposed test, server-running-p will return nil in this case,
> since it will find an auth file but no associated Emacs process.

It only will do that if the current, local server is trying to write
an authentication file in the NFS server. Else, it won't even *find*
the authentication file, so it will write its own, and start a new,
local server with a random port that won't collide with the NFS
server's Emacs server.

Still, the point is, either we try to do a complicate,
potentially-network-delaying check, or we opt for a simple,
not-totally-reliable check. I don't mind one way or the other (though
I won't be writing the complex one), but we should bear in mind that
even if the check fails, consequences are hardly catastrophic.
Currently we don't even check, just clobber the auth file. So any
partial fix is a net benefit, as long as we rename the function and/or
write a clear docstring stating that it cannot be assumed to be a
reliable indicator of the presence of another Emacs instance.

             Juanma

Re: error in server-running-p on M$

[Ulrich Mueller]

>>>>> On Sat, 22 Nov 2008, Juanma Barranquero wrote:

>> With the proposed test, server-running-p will return nil in this case,
>> since it will find an auth file but no associated Emacs process.

> It only will do that if the current, local server is trying to write
> an authentication file in the NFS server.

If you assume that the user's home directory is NFS mounted, then this
is the normal case.

> Still, the point is, either we try to do a complicate,
> potentially-network-delaying check, or we opt for a simple,
> not-totally-reliable check.

Yeah, I see the problem that one wouldn't like to have network
timeouts here ...

> I don't mind one way or the other (though I won't be writing the
> complex one), but we should bear in mind that even if the check
> fails, consequences are hardly catastrophic. Currently we don't even
> check, just clobber the auth file. So any partial fix is a net
> benefit, as long as we rename the function and/or write a clear
> docstring stating that it cannot be assumed to be a reliable
> indicator of the presence of another Emacs instance.

Agreed, of course a partial fix is better than not checking at all.
And I would vote against renaming the function, since I think its
intention is to find out if a server is running. Even if the
implementation (at the time being?) uses some heuristic that doesn't
cover 100% of all cases.

Another question, is the plan now to call server-running-p from
server-start (and refuse to start if it is returning non-nil)?

Ulrich

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sat, Nov 22, 2008 at 17:52, Ulrich Mueller <ulm@kph.uni-mainz.de> wrote:

> If you assume that the user's home directory is NFS mounted, then this
> is the normal case.

It would be the normal case that the user, on the local machine, would
have the home directory in a NFS server, and at the same time, there
would be an Emacs on the NFS server using an authentication file on
the user's home directory? (That's the case you mentioned before, if I
understand it correctly.) If that really happens, that should be fixed
at another level...

> Agreed, of course a partial fix is better than not checking at all.

OK.

> And I would vote against renaming the function, since I think its
> intention is to find out if a server is running.

I don't really care, as long as the user isn't mislead. But I'm not
sure the intention of the function is know whether a server is
running; more like a helper to avoid two or more Emacs to trample one
onto another, I think.

> Another question, is the plan now to call server-running-p from
> server-start (and refuse to start if it is returning non-nil)?

That's what Stefan said: "Another option is for server-start to try
and detect that there's already an active socket (using
server-running-p) and refuse to start."

             Juanma

Re: error in server-running-p on M$

[dhruva]

Hi,

On Sun, Nov 23, 2008 at 3:17 AM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Sat, Nov 22, 2008 at 17:52, Ulrich Mueller <ulm@kph.uni-mainz.de> wrote:
>
>> If you assume that the user's home directory is NFS mounted, then this
>> is the normal case.
>
> It would be the normal case that the user, on the local machine, would
> have the home directory in a NFS server, and at the same time, there
> would be an Emacs on the NFS server using an authentication file on
> the user's home directory? (That's the case you mentioned before, if I
> understand it correctly.) If that really happens, that should be fixed
> at another level...

How about storing the machine IP address and the port in the file for
each successful emacs server instance. The file name itself can be
machine IP DOT port. This will handle the case of NFS mounted home
folders.
Another option, how about having a user defined fixed port for running
emacs server. This might lead to a DOS attack but the port is defined
by the user and hence not something that can be guessed easily.

-dhruva

-- 
Contents reflect my personal views only!

Re: error in server-running-p on M$

[Stefan Monnier]

> Shouldn't it try to actually connect to the socket (in the
> server-use-tcp case), to find out if the server is functional?
> That would also be consistent with the behaviour for local sockets.

Yes, I think so.  It's the whole point of restricting it to 127.0.0.1
(so that trying to connect to it doesn't risk suffering a timeout).


        Stefan

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 04:56, dhruva <dhruvakm@gmail.com> wrote:

> How about storing the machine IP address and the port in the file for
> each successful emacs server instance. The file name itself can be
> machine IP DOT port. This will handle the case of NFS mounted home
> folders.

I don't understand. What do you gain with this? How does emacsclient
know with which instance must connect?

> Another option, how about having a user defined fixed port for running
> emacs server. This might lead to a DOS attack but the port is defined
> by the user and hence not something that can be guessed easily.

That was explicitly rejected while designing the TCP support for
emacsclient/server.

  Juanma

Re: error in server-running-p on M$

[Stefan Monnier]

> It does not detect (even now, for local sockets) whether another
> instance is running; it only detects whether another instance is
> running _with_the_same_server_setup_ (presumably because the intention
> is not disturbing it).

All of the server-* functions take server-name as an implicit argument
(that's what a global variable is, after all).
server-running-p is no exception and is not special in this respect.


        Stefan

Re: error in server-running-p on M$

[dhruva]

Hi,

On Sun, Nov 23, 2008 at 9:29 AM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Sun, Nov 23, 2008 at 04:56, dhruva <dhruvakm@gmail.com> wrote:
>
>> How about storing the machine IP address and the port in the file for
>> each successful emacs server instance. The file name itself can be
>> machine IP DOT port. This will handle the case of NFS mounted home
>> folders.
>
> I don't understand. What do you gain with this? How does emacsclient
> know with which instance must connect?

The host on which emacsclient is executed will look for a file with
the machine's IP. If the file exists, it picks the port from the file
extension part.

-dhruva

-- 
Contents reflect my personal views only!

Re: error in server-running-p on M$

[Stefan Monnier]

> Also, if the server authentication file says that Emacs process PID is
> using port P, and there's really an Emacs of pid PID but it is no
> longer answering to requests on port P, that is PID's problem, not the
> current instance's.

I prefer determining the liveness of the server by connecting to the
port rather than by looking at the list of running processes.

But, yes, it's true that there's a risk that the server died a while ago
and port P got reassigned in the mean time to something
completely different.  Hmmm.... so maybe it's better to check the PID is
still live :-( ?

        Stefan

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 05:01, dhruva <dhruvakm@gmail.com> wrote:

> The host on which emacsclient is executed will look for a file with
> the machine's IP. If the file exists, it picks the port from the file
> extension part.

Why do you suppose that when I run emacsclient on machine A I want to
connect to an Emacs server running on A with preference to another
running on B?

  Juanma

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 05:02, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> But, yes, it's true that there's a risk that the server died a while ago
> and port P got reassigned in the mean time to something
> completely different.  Hmmm.... so maybe it's better to check the PID is
> still live :-( ?

Not just that, but, if Emacs instance 1 is really running, why do we
want to check from instance 2? To be sure that instance 1 still has a
valid server accessible from emacsclient, or to avoid clobbering 1's
auth file? IMHO, it's no business of 2 to verify that 1 is still
answering, but just to be sure that it is not it, 2, the one
responsible of leaving 1 incommunicado.

  Juanma

Re: error in server-running-p on M$

[dhruva]

On Sun, Nov 23, 2008 at 9:33 AM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Sun, Nov 23, 2008 at 05:01, dhruva <dhruvakm@gmail.com> wrote:
>
>> The host on which emacsclient is executed will look for a file with
>> the machine's IP. If the file exists, it picks the port from the file
>> extension part.
>
> Why do you suppose that when I run emacsclient on machine A I want to
> connect to an Emacs server running on A with preference to another
> running on B?

emacsclient needs to use the '-f' option if it wants to use a specific
server. An automatic fallback would be to use local IP or local
machine IP. Does current emacs support running server that can accept
remote connections?

-dhruva

-- 
Contents reflect my personal views only!

Re: error in server-running-p on M$

[Juanma Barranquero]

> All of the server-* functions take server-name as an implicit argument
> (that's what a global variable is, after all).
> server-running-p is no exception and is not special in this respect.

I'm not sure that contradicts what I'm saying: that Emacs, when run
twice with the same setup, clobbers the auth file. Of course if the
user takes care of passing the right server-name he can make it all
work fine.

             Juanma

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 05:10, dhruva <dhruvakm@gmail.com> wrote:

> emacsclient needs to use the '-f' option if it wants to use a specific
> server.

Yes, unless the server is called "server" and the server
authentication file is at its default location, ~/.emacs.d/server/

> An automatic fallback would be to use local IP or local
> machine IP.

That "automatic fallback" is the one above: defaulting to "server".

> Does current emacs support running server that can accept
> remote connections?

  server-host is a variable defined in `server.el'.
  Its value is nil

  Documentation:
  The name or IP address to use as host address of the server process.
  If set, the server accepts remote connections; otherwise it is local.

Juanma

Re: error in server-running-p on M$

[dhruva]

Hi,

On Sun, Nov 23, 2008 at 9:45 AM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Sun, Nov 23, 2008 at 05:10, dhruva <dhruvakm@gmail.com> wrote:
>  server-host is a variable defined in `server.el'.
>  Its value is nil
>
>  Documentation:
>  The name or IP address to use as host address of the server process.
>  If set, the server accepts remote connections; otherwise it is local.

Thanks, I should have read the doc first before asking and I apologize
for that, just after sending the mail, I saw the server.el and
realized. Let me explore further if there is a simpler way to handle
this.

-dhruva

-- 
Contents reflect my personal views only!

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 05:17, dhruva <dhruvakm@gmail.com> wrote:

> Let me explore further if there is a simpler way to handle this.

If you want to propose changes to the way the TCP connections and
authentication files are used, please do it in a way compatible with
22.X...

  Juanma

Re: error in server-running-p on M$

[Stefan Monnier]

> I'm not sure that contradicts what I'm saying: that Emacs, when run
> twice with the same setup, clobbers the auth file. Of course if the

Assuming that server-start is changed to signal an error if
server-running-p returns non-nil, could you show us again the scenario
where the auth file is clobbered and in what way it's clobbered?


        Stefan

Re: error in server-running-p on M$

[Stefan Monnier]

> Not just that, but, if Emacs instance 1 is really running, why do we
> want to check from instance 2? To be sure that instance 1 still has a
> valid server accessible from emacsclient, or to avoid clobbering 1's
> auth file?

To avoid clobbering 1's auth file.


        Stefan

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 06:21, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> To avoid clobbering 1's auth file.

Then, it is not really necessary to connect with 1's server. Just the
fact that there's a local Emacs with the same PID that the one saved
in the authentication file, and acting as a local server, should be
reason enough for 2 to refuse overwriting the auth file.

> Assuming that server-start is changed to signal an error if
> server-running-p returns non-nil, could you show us again the scenario
> where the auth file is clobbered and in what way it's clobbered?

No, I've never claimed that. If server-start signals an error when
`server-running-p' returns non-nil, the auth file won't be clobbered.
(At least, in normal configurations; I'm not sure that is impossible
for some combination of non local servers, remote home user
directories, etc. That's left as an exercise for the people who
usually works in such environments ;-)

But we're trying to fix the easy, common problem first, aren't we?

             Juanma

Re: error in server-running-p on M$

[Stefan Monnier]

>> To avoid clobbering 1's auth file.
> Then, it is not really necessary to connect with 1's server. Just the
> fact that there's a local Emacs with the same PID that the one saved
> in the authentication file, and acting as a local server, should be
> reason enough for 2 to refuse overwriting the auth file.

As explained earlier, I thought it would be better to try and connect
than to try and check the PID, but I now think you're right: checking
the PID is probably a simpler solution (at least if stopping the server
properly removes the entry from the file).

>> Assuming that server-start is changed to signal an error if
>> server-running-p returns non-nil, could you show us again the scenario
>> where the auth file is clobbered and in what way it's clobbered?
> No, I've never claimed that.

Great.


        Stefan

Server protocol (was: Re: error in server-running-p on M$)

[Ulrich Mueller]

>>>>> On Sun, 23 Nov 2008, Juanma Barranquero wrote:

>> Let me explore further if there is a simpler way to handle this.

> If you want to propose changes to the way the TCP connections and
> authentication files are used, please do it in a way compatible with
> 22.X...

It is incompatible already:

Client 22 connecting to server 23:

   $ emacsclient-emacs-22 foo
   Waiting for Emacs...
   -error Unknown&_command:&_/local/home/ulm/foo

Client 23 connecting to server 22:

   $ emacsclient-emacs-23 foo
   Waiting for Emacs...
   *ERROR*: Unknown message: Authentication failed

The above is with TCP. With Unix sockets it is much worse, because
the connection succeeds but the information sent by the client is
misinterpreted by the server, resulting in several garbage buffers
(the following is for client 23 connecting to server 22):

,----
| CRM Buffer                Size  Mode              File
| .%  -current-frame           0  Fundamental       /dev/pts/-current-frame
|     *scratch*                0  Lisp Interaction
|   * *Messages*             302  Fundamental
|     foo                      6  Fundamental       ~/foo
|     -file                    0  Fundamental       ~/-file
|     xterm                    0  Fundamental       ~/xterm
|  %  2                        0  Fundamental       /dev/pts/2
|  %  -tty                     0  Fundamental       /dev/pts/-tty
`----

This makes me wonder if the client shouldn't send some token
identifying the protocol version, which would be checked by the
server.

Ulrich

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 06:57, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> but I now think you're right: checking
> the PID is probably a simpler solution (at least if stopping the server
> properly removes the entry from the file).

Stopping the server removes the file.

  Juanma

Re: Server protocol (was: Re: error in server-running-p on M$)

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 09:14, Ulrich Mueller <ulm@kph.uni-mainz.de> wrote:

> It is incompatible already:

Yes, unfortunately. The multi-tty code changed emacsclient's protocol
considerably.

I was talking about the user perspective. I have, for example, a bunch
of code that depends on the server auth file and its relation with
server-name. That would be invalidated if suddenly we switched to
other kind of authentication method. Not the end of the world, but
there should be clear benefits before doing that.

> This makes me wonder if the client shouldn't send some token
> identifying the protocol version, which would be checked by the
> server.

Perhaps.

             Juanma

Re: error in server-running-p on M$

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 12:26, Juanma Barranquero <lekktu@gmail.com> wrote:

> Stopping the server removes the file.

Ah, no, it doesn't, because IIRC you opposed to it:

  ;; Delete the associated connection file, if applicable.
  ;; This is actually problematic: the file may have been overwritten by
  ;; another Emacs server in the mean time, so it's not ours any more.
  ;; (and (process-contact proc :server)
  ;;      (eq (process-status proc) 'closed)
  ;;      (ignore-errors (delete-file (process-get proc :server-file))))

I was mistaken because my .emacs includes code to delete it.

But it *should* be deleted.

  Juanma

Re: error in server-running-p on M$

[Ulrich Mueller]

>>>>> On Sun, 23 Nov 2008, Juanma Barranquero wrote:

>> Stopping the server removes the file.

> Ah, no, it doesn't, because IIRC you opposed to it:

>   ;; Delete the associated connection file, if applicable.
>   ;; This is actually problematic: the file may have been overwritten by
>   ;; another Emacs server in the mean time, so it's not ours any more.
>   ;; (and (process-contact proc :server)
>   ;;      (eq (process-status proc) 'closed)
>   ;;      (ignore-errors (delete-file (process-get proc :server-file))))

Read the file back and only delete it if it contains our own process
id? Of course there is a race condition ...

Ulrich

Re: Server protocol (was: Re: error in server-running-p on M$)

[dhruva]

Hi,

On Sun, Nov 23, 2008 at 5:03 PM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Sun, Nov 23, 2008 at 09:14, Ulrich Mueller <ulm@kph.uni-mainz.de> wrote:
>
>> It is incompatible already:
>
> Yes, unfortunately. The multi-tty code changed emacsclient's protocol
> considerably.

Would daemon option replace the need for server mode from the
functionality perspective or there is nothing related between them?
 The only reason I use server mode is to open files in a single
running instance of emacs. I can do the same using daemon option on
non M$ platform though.

-dhruva

-- 
Contents reflect my personal views only!

Re: Server protocol (was: Re: error in server-running-p on M$)

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 17:33, dhruva <dhruvakm@gmail.com> wrote:

> Would daemon option replace the need for server mode from the
> functionality perspective or there is nothing related between them?

The daemon option *uses* server.el / emacsclient. And it is perfectly
reasonable to use server/emacsclient even if you're not interested in
having a daemonized Emacs.

             Juanma

Re: error in server-running-p on M$

[Stefan Monnier]

>> Stopping the server removes the file.
> Ah, no, it doesn't, because IIRC you opposed to it:

>   ;; Delete the associated connection file, if applicable.
>   ;; This is actually problematic: the file may have been overwritten by
>   ;; another Emacs server in the mean time, so it's not ours any more.

That's OK: when we add server-running-p to server-start this objection
will disappear so we can change the code at the same time to remove the
entry when stopping the server.


        Stefan

Re: error in server-running-p on M$

[Juanma Barranquero]

On Mon, Nov 24, 2008 at 03:55, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> That's OK: when we add server-running-p to server-start this objection
> will disappear so we can change the code at the same time to remove the
> entry when stopping the server.

WDYT about the following patch?

  Juanma



2008-11-24  Juanma Barranquero  <lekktu@gmail.com>

	* server.el (server-sentinel): Uncomment code to delete connection file.
	(server-running-p): Return t for TCP servers if the auth file says
	there's a local server and we find an Emacs process with the right PID.
	Doc fix.
	(server-start): Save the connection file in the process' property list.
	Delete it only when we are reasonably convinced that it is not owned by
	a running server.


Index: lisp/server.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/server.el,v
retrieving revision 1.175
diff -u -2 -b -r1.175 server.el
--- lisp/server.el	18 Nov 2008 16:27:09 -0000	1.175
+++ lisp/server.el	24 Nov 2008 14:40:52 -0000
@@ -326,9 +326,7 @@
     (set-process-query-on-exit-flag proc nil))
   ;; Delete the associated connection file, if applicable.
-  ;; This is actually problematic: the file may have been overwritten by
-  ;; another Emacs server in the mean time, so it's not ours any more.
-  ;; (and (process-contact proc :server)
-  ;;      (eq (process-status proc) 'closed)
-  ;;      (ignore-errors (delete-file (process-get proc :server-file))))
+  (and (process-contact proc :server)
+       (eq (process-status proc) 'closed)
+       (ignore-errors (delete-file (process-get proc :server-file))))
   (server-log (format "Status changed to %s: %s" (process-status
proc) msg) proc)
   (server-delete-client proc))
@@ -465,12 +463,14 @@
 	 (yes-or-no-p
 	  "The current server still has clients; delete them? "))
+    (let* ((server-dir (if server-use-tcp server-auth-dir server-socket-dir))
+	   (server-file (expand-file-name server-name server-dir)))
     (when server-process
       ;; kill it dead!
       (ignore-errors (delete-process server-process)))
     ;; Delete the socket files made by previous server invocations.
-    (when server-socket-dir
-      (condition-case ()
-	  (delete-file (expand-file-name server-name server-socket-dir))
-	(error nil)))
+      (if (not (server-running-p server-name))
+	  (ignore-errors (delete-file server-file))
+	(setq server-mode nil)  ;; already set by the minor mode code
+	(error "Server %S is already running" server-name))
     ;; If this Emacs already had a server, clear out associated status.
     (while server-clients
@@ -481,6 +481,4 @@
 	  (server-log (message "Server stopped"))
 	  (setq server-process nil))
-      (let* ((server-dir (if server-use-tcp server-auth-dir server-socket-dir))
-	     (server-file (expand-file-name server-name server-dir)))
 	;; Make sure there is a safe directory in which to place the socket.
 	(server-ensure-safe-dir server-dir)
@@ -517,4 +515,5 @@
 			       :plist '(:authenticated t)))))
 	  (unless server-process (error "Could not start server process"))
+	  (process-put server-process :server-file server-file)
 	  (when server-use-tcp
 	    (let ((auth-key
@@ -535,5 +534,7 @@

 (defun server-running-p (&optional name)
-  "Test whether server NAME is running."
+  "Test whether server NAME is running.
+NOTE: This function is intended to be called from `server-start'
+and it is NOT 100% reliable."
   (interactive
    (list (if current-prefix-arg
@@ -541,5 +542,15 @@
   (unless name (setq name server-name))
   (condition-case nil
-      (progn
+      (if server-use-tcp
+	  (with-temp-buffer
+	    (insert-file-contents-literally
+	     (expand-file-name name server-auth-dir))
+	    (and (looking-at "127\.0\.0\.1:[0-9]+ \\([0-9]+\\)")
+		 (let ((case-fold-search t)
+		       (proc (assq 'comm
+				   (system-process-attributes
+				    (string-to-number (match-string 1))))))
+		   (and proc
+			(string-match-p "emacs" (cdr proc))))))
 	(delete-process
 	 (make-network-process

Re: error in server-running-p on M$

[Ulrich Mueller]

>>>>> On Mon, 24 Nov 2008, Juanma Barranquero wrote:

> WDYT about the following patch?

> +      (if (not (server-running-p server-name))
> +          (ignore-errors (delete-file server-file))
> +        (setq server-mode nil)  ;; already set by the minor mode code
> +        (error "Server %S is already running" server-name))

Instead of a hard error, maybe you could add a yes-or-no-p
confirmation here? Or an additional (prefix?) argument to override the
test, since we know that server-running-p is not 100% reliable.

Ulrich

Re: error in server-running-p on M$

[Juanma Barranquero]

On Mon, Nov 24, 2008 at 16:10, Ulrich Mueller <ulm@kph.uni-mainz.de> wrote:

> Instead of a hard error, maybe you could add a yes-or-no-p
> confirmation here?

No objection here.

> Or an additional (prefix?) argument to override the
> test, since we know that server-running-p is not 100% reliable.

For server-start, you mean? It already has a prefix arg (as does
server-mode, obviously).

  Juanma

Re: error in server-running-p on M$

[Ulrich Mueller]

>>>>> On Mon, 24 Nov 2008, Juanma Barranquero wrote:

>> Instead of a hard error, maybe you could add a yes-or-no-p
>> confirmation here?

> No objection here.

>> Or an additional (prefix?) argument to override the
>> test, since we know that server-running-p is not 100% reliable.

> For server-start, you mean?

Yes. My thought was that in daemon mode server-start is called
non-interactively, so an interactive confirmation is useless there.

> It already has a prefix arg (as does server-mode, obviously).

So another optional argument might be needed, with three possible
values for "override", "don't override", "ask for confirmation".

Ulrich

Re: error in server-running-p on M$

[Juanma Barranquero]

On Mon, Nov 24, 2008 at 16:34, Ulrich Mueller <ulm@kph.uni-mainz.de> wrote:

> Yes. My thought was that in daemon mode server-start is called
> non-interactively, so an interactive confirmation is useless there.

That's not a prefix argument, then. You're not going to call
`server-start' or `server-mode' interactively from a daemonized Emacs;
the call happens in startup.el. Perhaps a variable (or defcustom)?

> So another optional argument might be needed, with three possible
> values for "override", "don't override", "ask for confirmation".

I think that's a bit messy.

  Juanma

Re: error in server-running-p on M$

[Stefan Monnier]

> WDYT about the following patch?

Looks good, tho see comment below.

>    ;; Delete the associated connection file, if applicable.
> -  ;; This is actually problematic: the file may have been overwritten by
> -  ;; another Emacs server in the mean time, so it's not ours any more.
> -  ;; (and (process-contact proc :server)
> -  ;;      (eq (process-status proc) 'closed)
> -  ;;      (ignore-errors (delete-file (process-get proc :server-file))))
> +  (and (process-contact proc :server)
> +       (eq (process-status proc) 'closed)
> +       (ignore-errors (delete-file (process-get proc :server-file))))
>    (server-log (format "Status changed to %s: %s" (process-status

It would be good to keep some comment about the fact that we don't know
for sure that the file is ours.

> +      (if (not (server-running-p server-name))
> +	  (ignore-errors (delete-file server-file))
> +	(setq server-mode nil)  ;; already set by the minor mode code
> +	(error "Server %S is already running" server-name))

As Ulrich points out, we should make sure that we can always somehow
start a server.

> -  "Test whether server NAME is running."
> +  "Test whether server NAME is running.
> +NOTE: This function is intended to be called from `server-start'
> +and it is NOT 100% reliable."

Just saying it's unreliable is not sufficient.  We need to give some
guarantee about what kind of incorrect result we may get.  E.g. it could
return t, nil, or `not-sure'.
Also we should describe the reason for the unreliability: we prefer
giving an immediate response over risking non-termination.

Finally, for the purpose of server-start, we don't necessarily care if
there is a server named "foo" running on some other machine, so maybe in
the server-use-tcp case, if the IP is not 127.0.0.1, server-running-p
might return a special value like `remote' and server-start might treat
it like "not running" (i.e. it will remove the server-file).


        Stefan

Re: error in server-running-p on M$

[Chetan Pandya]

I haven't seen earlier messages, but I am glad something is being done here.



--- On Tue, 12/9/08, Stefan Monnier <monnier@IRO.UMontreal.CA> wrote:


> Looks good, tho see comment below.
> 
> > +  (and (process-contact proc :server)
> > +       (eq (process-status proc) 'closed)
> > +       (ignore-errors (delete-file (process-get proc
> :server-file))))
> >    (server-log (format "Status changed to %s:
> %s" (process-status
> 
> It would be good to keep some comment about the fact that we don't know
> for sure that the file is ours.

It isn't too difficult to check that. Here is a version that checks that the file has the same pid.

(let ((server-file (process-get proc :server-file)))
  (with-temp-buffer
    (set-buffer-multibyte nil)
    (insert-file server-file)
    (beginning-of-buffer)
    (end-of-line)
    (skip-chars-backward "0-9")
    (if (eq (read (current-buffer)) (emacs-pid))
	(ignore-errors (delete-file server-file)))))


> > +      (if (not (server-running-p server-name))
> > +	  (ignore-errors (delete-file server-file))
> > +	(setq server-mode nil)  ;; already set by the minor
> mode code
> > +	(error "Server %S is already running"
> server-name))
 
> Finally, for the purpose of server-start, we don't
> necessarily care if
> there is a server named "foo" running on some
> other machine, so maybe in
> the server-use-tcp case, if the IP is not 127.0.0.1,
> server-running-p
> might return a special value like `remote' and
> server-start might treat
> it like "not running" (i.e. it will remove the
> server-file).

It is possible for the IP to be 127.0.0.1 and still the server be running on another system.

Chetan

Re: error in server-running-p on M$

[Juanma Barranquero]

On Tue, Dec 9, 2008 at 21:46, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> As Ulrich points out, we should make sure that we can always somehow
> start a server.

OK, but how do you want to do this? Another argument to server-start?
A configuration variable? What about interactive use?

> [...] E.g. it could
> return t, nil, or `not-sure'.
> [...]
> Finally, for the purpose of server-start, we don't necessarily care if
> there is a server named "foo" running on some other machine, so maybe in
> the server-use-tcp case, if the IP is not 127.0.0.1, server-running-p
> might return a special value like `remote' and server-start might treat
> it like "not running" (i.e. it will remove the server-file).

So you're saying:

 t -> the server file cannot be deleted; there's a running server
 nil -> the server file can be deleted; there's no running server
 remote -> the server file can be deleted; ignore any possible remote TCP server
 not-sure -> ????

This is getting confusing, so let's sort the cases:

 1) local socket, client process works => sure, server -> t
 2) local socket, client process fails => sure, no server -> nil
 3) TCP sockets, no server file => sure, no server -> nil
 4) TCP sockets, not 127.0.0.1 => not sure -> remote
 5) TCP sockets, 127.0.0.1, no process with PID => not sure (could be
shared dir) -> not-sure
 6) TCP sockets, 127.0.0.1, process PID, no "emacs" -> not sure (like
above) -> not-sure
 7) TCP sockets, 127.0.0.1, process PID, "emacs" -> sure (sort of) -> t

Now the `t' cases are clear and safe (don't delete the server file),
and the `nil' ones are too (delete the file, if exists).

You're suggesting that 4 should return `remote' and be treated like
`nil' (delete). But 5 and 6 could also be remote, and they must also
be treated like `nil' (delete), or they won't be useful at all. So,
there's no real distinction between `remote' and `not-sure', and
`remote' does not even carry useful information (we could return
not-sure for some remote servers).

Now, to be really safe, 5) should be not-sure, but delete, and 6)
should be not-sure, and don't delete (because Emacs could be running
with another name).

Thoughts?

    Juanma

Re: error in server-running-p on M$

[Stefan Monnier]

>> As Ulrich points out, we should make sure that we can always somehow
>> start a server.
> OK, but how do you want to do this? Another argument to server-start?
> A configuration variable? What about interactive use?

A config var doesn't sound right.  Programmatically, there's no need to
do anything special.  The problem is for interactive use.  I don't have
a good solution to propose just now, we could just say "to force-start
a sever first do M-x server-force-delete and then M-x server-mode".

> This is getting confusing, so let's sort the cases:

>  1) local socket, client process works => sure, server -> t
>  2) local socket, client process fails => sure, no server -> nil
>  3) TCP sockets, no server file => sure, no server -> nil
>  4) TCP sockets, not 127.0.0.1 => not sure -> remote
>  5) TCP sockets, 127.0.0.1, no process with PID => not sure (could be
> shared dir) -> not-sure
>  6) TCP sockets, 127.0.0.1, process PID, no "emacs" -> not sure (like
> above) -> not-sure
>  7) TCP sockets, 127.0.0.1, process PID, "emacs" -> sure (sort of) -> t

> Now the `t' cases are clear and safe (don't delete the server file),
> and the `nil' ones are too (delete the file, if exists).

> You're suggesting that 4 should return `remote' and be treated like
> `nil' (delete). But 5 and 6 could also be remote, and they must also
> be treated like `nil' (delete), or they won't be useful at all. So,
> there's no real distinction between `remote' and `not-sure', and
> `remote' does not even carry useful information (we could return
> not-sure for some remote servers).

> Now, to be really safe, 5) should be not-sure, but delete, and 6)
> should be not-sure, and don't delete (because Emacs could be running
> with another name).

> Thoughts?

That sounds about right.  The doc could simply say "t, nil, other" where
"other" means that we're not sure and the value returned might give
a hint about why we're not sure.

As for process name, I'd rather not check the name of the process.


        Stefan

Re: error in server-running-p on M$

[Juanma Barranquero]

On Wed, Dec 10, 2008 at 19:30, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> A config var doesn't sound right.  Programmatically, there's no need to
> do anything special.  The problem is for interactive use. I don't have
> a good solution to propose just now, we could just say "to force-start
> a sever first do M-x server-force-delete and then M-x server-mode".

If you're not going to worry about the non-interactive use, there's no
need of using another command. Ulrich's original proposal, asking the
user, would be better IMHO.  (Of course, noninteractive == t would
turn it off.)

> That sounds about right.  The doc could simply say "t, nil, other" where
> "other" means that we're not sure and the value returned might give
> a hint about why we're not sure.

Sorry, I don't understand. Do you mean that the documentation would
say "t, nil, other", but we would return t, nil, and other values that
give a hint (like not-sure, or remote), or do you mean that we return
t, nil, other, and the caller must know what to do? I prefer the
second option.

> As for process name, I'd rather not check the name of the process.

I'd guess (out of the blue sky) than in a heavily used system the
probability of a process ID collision between a long-dead process and
a new one is significant. I'd feel safer checking the name and adding
a note to the docstring about the dangers of running Emacs servers
with atypical names. But it is your call, I'll implement what you
decide.

    Juanma

Re: error in server-running-p on M$

[Stefan Monnier]

>> A config var doesn't sound right.  Programmatically, there's no need to
>> do anything special.  The problem is for interactive use. I don't have
>> a good solution to propose just now, we could just say "to force-start
>> a sever first do M-x server-force-delete and then M-x server-mode".

> If you're not going to worry about the non-interactive use, there's no
> need of using another command. Ulrich's original proposal, asking the
> user, would be better IMHO.  (Of course, noninteractive == t would
> turn it off.)

I usually prefer it if the command just fails and lets the user run some
other command to do what she wants.  Sometimes asking the question is
a better option, but here I don't thinkg that its worth it.  The main
problem with asking a question is that it's modal.

>> That sounds about right.  The doc could simply say "t, nil, other" where
>> "other" means that we're not sure and the value returned might give
>> a hint about why we're not sure.

> Sorry, I don't understand. Do you mean that the documentation would
> say "t, nil, other", but we would return t, nil, and other values that
> give a hint (like not-sure, or remote), or do you mean that we return
> t, nil, other, and the caller must know what to do? I prefer the
> second option.

Second option is fine.

>> As for process name, I'd rather not check the name of the process.
> I'd guess (out of the blue sky) than in a heavily used system the
> probability of a process ID collision between a long-dead process and
> a new one is significant. I'd feel safer checking the name and adding
> a note to the docstring about the dangers of running Emacs servers
> with atypical names. But it is your call, I'll implement what you
> decide.

Most/all Unix locks based on process-ids (like the ones used by Emacs,
for example) don't pay attention to the process name.  So experience
shows it's usually good enough.
We can also reduce the likelihood of leaving behind some obsolete
socket/file using kill-emacs-hook.


        Stefan

Re: error in server-running-p on M$

[Juanma Barranquero]

On Thu, Dec 11, 2008 at 03:07, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I usually prefer it if the command just fails and lets the user run some
> other command to do what she wants.  Sometimes asking the question is
> a better option, but here I don't thinkg that its worth it.  The main
> problem with asking a question is that it's modal.

I've implemented `server-force-delete', as you suggested.

> Most/all Unix locks based on process-ids (like the ones used by Emacs,
> for example) don't pay attention to the process name.  So experience
> shows it's usually good enough.

OK. Now `server-running-p' will return t for a matching PID process,
and does not check the name. It is the safer behavior anyway.

> We can also reduce the likelihood of leaving behind some obsolete
> socket/file using kill-emacs-hook.

When the server is running, `kill-emacs-hook' already contains code to
turn the server off; (with the patch) that deletes the connection
file.

Please, take a look at the attached patch.

    Juanma



2008-12-11  Juanma Barranquero  <lekktu@gmail.com>

	* server.el (server-sentinel): Uncomment code to delete connection file.
	(server-start): Save the connection file in the server property list.
	Delete it only when we are reasonably convinced that it is not owned by
	a running server.
	(server-force-delete): New command to force-delete the connection file.
	(server-running-p): Return t also for local TCP servers when we find a
	process with a matching PID, and :other for undecided cases.



Index: lisp/server.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/server.el,v
retrieving revision 1.175
diff -u -3 -b -r1.175 server.el
--- lisp/server.el	18 Nov 2008 16:27:09 -0000	1.175
+++ lisp/server.el	11 Dec 2008 16:15:17 -0000
@@ -327,9 +327,9 @@
   ;; Delete the associated connection file, if applicable.
   ;; This is actually problematic: the file may have been overwritten by
   ;; another Emacs server in the mean time, so it's not ours any more.
-  ;; (and (process-contact proc :server)
-  ;;      (eq (process-status proc) 'closed)
-  ;;      (ignore-errors (delete-file (process-get proc :server-file))))
+  (and (process-contact proc :server)
+       (eq (process-status proc) 'closed)
+       (ignore-errors (delete-file (process-get proc :server-file))))
   (server-log (format "Status changed to %s: %s" (process-status
proc) msg) proc)
   (server-delete-client proc))

@@ -458,20 +458,26 @@
 Emacs distribution as your standard \"editor\".

 Optional argument LEAVE-DEAD (interactively, a prefix arg) means just
-kill any existing server communications subprocess."
+kill any existing server communications subprocess.
+
+If `server-running-p' returns t, the server is not started.
+To force-start a server, do \\[server-force-delete] and then
+\\[server-start]."
   (interactive "P")
   (when (or
 	 (not server-clients)
 	 (yes-or-no-p
 	  "The current server still has clients; delete them? "))
+    (let* ((server-dir (if server-use-tcp server-auth-dir server-socket-dir))
+	   (server-file (expand-file-name server-name server-dir)))
     (when server-process
       ;; kill it dead!
       (ignore-errors (delete-process server-process)))
     ;; Delete the socket files made by previous server invocations.
-    (when server-socket-dir
-      (condition-case ()
-	  (delete-file (expand-file-name server-name server-socket-dir))
-	(error nil)))
+      (if (memq (server-running-p server-name) '(nil :other))
+	  (ignore-errors (delete-file server-file))
+	(setq server-mode nil) ;; already set by the minor mode code
+	(error "Server %S is already running" server-name))
     ;; If this Emacs already had a server, clear out associated status.
     (while server-clients
       (server-delete-client (car server-clients)))
@@ -480,12 +486,8 @@
 	(progn
 	  (server-log (message "Server stopped"))
 	  (setq server-process nil))
-      (let* ((server-dir (if server-use-tcp server-auth-dir server-socket-dir))
-	     (server-file (expand-file-name server-name server-dir)))
 	;; Make sure there is a safe directory in which to place the socket.
 	(server-ensure-safe-dir server-dir)
-	;; Remove any leftover socket or authentication file.
-	(ignore-errors (delete-file server-file))
 	(when server-process
 	  (server-log (message "Restarting server")))
 	(letf (((default-file-modes) ?\700))
@@ -516,6 +518,7 @@
 			       :service server-file
 			       :plist '(:authenticated t)))))
 	  (unless server-process (error "Could not start server process"))
+	  (process-put server-process :server-file server-file)
 	  (when server-use-tcp
 	    (let ((auth-key
 		   (loop
@@ -533,14 +536,43 @@
 			" " (int-to-string (emacs-pid))
 			"\n" auth-key)))))))))

+;;;###autoload
+(defun server-force-delete (&optional name)
+  "Unconditionally delete connection file for server NAME.
+NAME defaults to `server-name'.  With argument, ask for NAME."
+  (interactive
+   (list (if current-prefix-arg
+	     (read-string "Server name: " nil nil server-name))))
+  (let ((file (expand-file-name (or name server-name)
+				(if server-use-tcp
+				    server-auth-dir
+				  server-socket-dir))))
+    (condition-case nil
+	(progn
+	  (delete-file file)
+	  (message "Connection file %S deleted" file))
+      (file-error
+       (message "Connection file %S not found or not deleted" file)))))
+
 (defun server-running-p (&optional name)
-  "Test whether server NAME is running."
+  "Test whether server NAME is running.
+
+NOTE: This function is designed to return immediately, rather than
+risking non-termination.  In some cases it returns `:other' when it
+cannot completely determine whether there's a server running or not."
   (interactive
    (list (if current-prefix-arg
 	     (read-string "Server name: " nil nil server-name))))
   (unless name (setq name server-name))
   (condition-case nil
-      (progn
+      (if server-use-tcp
+	  (with-temp-buffer
+	    (insert-file-contents-literally (expand-file-name name server-auth-dir))
+	    (or (and (looking-at "127\.0\.0\.1:[0-9]+ \\([0-9]+\\)")
+		     (assq 'comm
+			   (system-process-attributes
+			    (string-to-number (match-string 1)))))
+		:other))
 	(delete-process
 	 (make-network-process
 	  :name "server-client-test" :family 'local :server nil :noquery t

Re: error in server-running-p on M$

[Stefan Monnier]

>> I usually prefer it if the command just fails and lets the user run some
>> other command to do what she wants.  Sometimes asking the question is
>> a better option, but here I don't thinkg that its worth it.  The main
>> problem with asking a question is that it's modal.

> I've implemented `server-force-delete', as you suggested.

>> Most/all Unix locks based on process-ids (like the ones used by Emacs,
>> for example) don't pay attention to the process name.  So experience
>> shows it's usually good enough.

> OK. Now `server-running-p' will return t for a matching PID process,
> and does not check the name. It is the safer behavior anyway.

>> We can also reduce the likelihood of leaving behind some obsolete
>> socket/file using kill-emacs-hook.

> When the server is running, `kill-emacs-hook' already contains code to
> turn the server off; (with the patch) that deletes the connection
> file.

> Please, take a look at the attached patch.

Looks good.  I've added some more nitpicks, with which you can install it.

>    ;; Delete the associated connection file, if applicable.
>    ;; This is actually problematic: the file may have been overwritten by
>    ;; another Emacs server in the mean time, so it's not ours any more.
> -  ;; (and (process-contact proc :server)
> -  ;;      (eq (process-status proc) 'closed)
> -  ;;      (ignore-errors (delete-file (process-get proc :server-file))))
> +  (and (process-contact proc :server)
> +       (eq (process-status proc) 'closed)
> +       (ignore-errors (delete-file (process-get proc :server-file))))

I didn't mean to leave the comment as is.  Please update the comment to
say that the file *should* be ours thanks to the server-wunning-p check
in server-start.

> +If `server-running-p' returns t, the server is not started.

I'd just write it as "If a server is already running, the server is not
started".

> +      (if (memq (server-running-p server-name) '(nil :other))

Better check (not (eq t (server-running-p server-name))).

> -	;; Remove any leftover socket or authentication file.

You've moved the subsequent line but left out this comment that goes
with it.

> +;;;###autoload
> +(defun server-force-delete (&optional name)
> +  "Unconditionally delete connection file for server NAME.
> +NAME defaults to `server-name'.  With argument, ask for NAME."
> +  (interactive
> +   (list (if current-prefix-arg
> +	     (read-string "Server name: " nil nil server-name))))
> +  (let ((file (expand-file-name (or name server-name)
> +				(if server-use-tcp
> +				    server-auth-dir
> +				  server-socket-dir))))
> +    (condition-case nil
> +	(progn
> +	  (delete-file file)
> +	  (message "Connection file %S deleted" file))
> +      (file-error
> +       (message "Connection file %S not found or not deleted" file)))))

This should first stop our own server.  The user will usually not run
it when our server is running, but she might do it occasionally.

>  (defun server-running-p (&optional name)
> -  "Test whether server NAME is running."
> +  "Test whether server NAME is running.
> +
> +NOTE: This function is designed to return immediately, rather than
> +risking non-termination.  In some cases it returns `:other' when it
> +cannot completely determine whether there's a server running or not."

The docstring should say:
nil => the server is definitely not running.
t   => the server seems to be running.
something else => we cannot determine whether it's running without
    using commands which may have to wait for a long time.

No need to document the :other value.

> +	    (or (and (looking-at "127\.0\.0\.1:[0-9]+ \\([0-9]+\\)")
> +		     (assq 'comm
> +			   (system-process-attributes
> +			    (string-to-number (match-string 1))))

                     t)

> +		:other))

-- Stefan

Re: error in server-running-p on M$

[Juanma Barranquero]

On Thu, Dec 11, 2008 at 19:47, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I didn't mean to leave the comment as is.  Please update the comment to
> say that the file *should* be ours thanks to the server-wunning-p check
> in server-start.

I was trying to weasel out of writing it, but you caught me ;-)

>> -     ;; Remove any leftover socket or authentication file.
>
> You've moved the subsequent line but left out this comment that goes
> with it.

It was not an oversight, I didn't think it was needed. I've moved it, though.

> This should first stop our own server.  The user will usually not run
> it when our server is running, but she might do it occasionally.

I've done that, and now I still dislike `server-force-delete' even
more. I strongly think that a simple question on server-start would
have been cleaner.

Currently the function must do

  (server-mode -1)  ;; which deletes the connection file, because of
the process sentinel
  (condition-case nil
      (delete-file ...)
      ...)

which is kind of redundant. It almost should be

  (if server-mode
       (server-mode -1)
     (condition-case nil
         ...))

but the name of the current server could be different of the one for
the connection file we're trying to delete.

Ugly IMHO.

>> +         (or (and (looking-at "127\.0\.0\.1:[0-9]+ \\([0-9]+\\)")
>> +                  (assq 'comm
>> +                        (system-process-attributes
>> +                         (string-to-number (match-string 1))))
>
>                     t)

Yes, a silly oversight. Thanks.

The code is now committed.

    Juanma

Re: error in server-running-p on M$

[Stefan Monnier]

>> This should first stop our own server.  The user will usually not run
>> it when our server is running, but she might do it occasionally.

> I've done that, and now I still dislike `server-force-delete' even
> more. I strongly think that a simple question on server-start would
> have been cleaner.

For a function that's likely to be called from .emacs, I think it's
better to avoid asking questions as far as possible.

> which is kind of redundant. It almost should be

>   (if server-mode
>        (server-mode -1)
>      (condition-case nil
>          ...))

I don't see why it's a problem.

> but the name of the current server could be different of the one for
> the connection file we're trying to delete.
> Ugly IMHO.

BTW, another option is to let server-stop ask the question, so
C-u -1 M-x server-mode would provide the functionality  of
server-force-delete: if server is running, turn it off, if it's not
running and there's no socker/server file, don't do anything, and if
there is some file ask the user whether to force-delete it.

> The code is now committed.

Thanks,


        Stefan

Re: error in server-running-p on M$

[Juanma Barranquero]

On Fri, Dec 12, 2008 at 05:51, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> BTW, another option is to let server-stop ask the question, so
> C-u -1 M-x server-mode would provide the functionality  of
> server-force-delete: if server is running, turn it off, if it's not
> running and there's no socker/server file, don't do anything, and if
> there is some file ask the user whether to force-delete it.

Let's see what people thinks of the current interface. I'm sure if
there is a better one (perhaps the one you're suggesting) we will know
soon enough.

    Juanma

Re: error in server-running-p on M$

[Stefan Monnier]

>> BTW, another option is to let server-stop ask the question, so
>> C-u -1 M-x server-mode would provide the functionality  of
>> server-force-delete: if server is running, turn it off, if it's not
>> running and there's no socker/server file, don't do anything, and if
>> there is some file ask the user whether to force-delete it.

> Let's see what people thinks of the current interface. I'm sure if
> there is a better one (perhaps the one you're suggesting) we will know
> soon enough.

Actually, it seems like a sufficiently rarely-used functionality that it
probably won't show up until much later,


        Stefan

Re: error in server-running-p on M$

[Juanma Barranquero]

On Fri, Dec 12, 2008 at 19:49, Stefan Monnier <monnier@iro.umontreal.ca> wrote:


> Actually, it seems like a sufficiently rarely-used functionality that it
> probably won't show up until much later,

We haven't started pretest yet. Your "much later" is my "soon enough" :-)

    Juanma