From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: error in server-running-p on M$ Date: Thu, 11 Dec 2008 13:47:12 -0500 Message-ID: References: NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1229021373 31154 80.91.229.12 (11 Dec 2008 18:49:33 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 11 Dec 2008 18:49:33 +0000 (UTC) Cc: Eli Zaretskii , Ulrich Mueller , emacs-devel@gnu.org To: "Juanma Barranquero" Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Dec 11 19:50:36 2008 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1LAqc8-0006JY-DO for ged-emacs-devel@m.gmane.org; Thu, 11 Dec 2008 19:50:20 +0100 Original-Received: from localhost ([127.0.0.1]:59403 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LAqax-00085o-0p for ged-emacs-devel@m.gmane.org; Thu, 11 Dec 2008 13:49:07 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LAqZA-00063y-Vn for emacs-devel@gnu.org; Thu, 11 Dec 2008 13:47:17 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LAqZA-00062h-9H for emacs-devel@gnu.org; Thu, 11 Dec 2008 13:47:16 -0500 Original-Received: from [199.232.76.173] (port=46295 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LAqZ9-00062U-Ve for emacs-devel@gnu.org; Thu, 11 Dec 2008 13:47:16 -0500 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.182]:29551) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LAqZ7-00012a-EO; Thu, 11 Dec 2008 13:47:13 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AkcFAHvuQElLd+2g/2dsb2JhbACBbMxfgnmBLA X-IronPort-AV: E=Sophos;i="4.36,206,1228107600"; d="scan'208";a="30933371" Original-Received: from 75-119-237-160.dsl.teksavvy.com (HELO pastel.home) ([75.119.237.160]) by ironport2-out.teksavvy.com with ESMTP; 11 Dec 2008 13:47:12 -0500 Original-Received: by pastel.home (Postfix, from userid 20848) id 7FB8386F6; Thu, 11 Dec 2008 13:47:12 -0500 (EST) In-Reply-To: (Juanma Barranquero's message of "Thu, 11 Dec 2008 17:30:27 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux) X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:106813 Archived-At: >> 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