bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

By default, desktop-save-mode will not save (and will set
desktop-dirname to nil) if a lock file exists with a PID in it.

It's problematic however when Emacs gets killed and does not release the
lock.  Upon next start, desktop-save-mode will refuse to save because
the lock exists, even though no process is using it.  In terms of user
experience, it's pretty bad considering the error feedback is just one
line in the *Messages* buffer (I almost never notice it when it happens)
and the problem is persistent across reboots (the lock file will remain
as long as the user does not remove it manually).

Furthermore, isn't it strange to just check if there lock file contains
a number and not actually check if it's an existing PID?

I think it would make much more sense to actually check if the owner is
an _existing_ process named "emacs".

Here is my suggested patch:

	(defun desktop-owner (&optional dirname)
	  "Return the PID of the Emacs process that owns the desktop file in DIRNAME.
	Return nil if no desktop file found or no Emacs process is using it.
	DIRNAME omitted or nil means use `desktop-dirname'."
	  (let (owner
	        (file (desktop-full-lock-name dirname)))
	    (and (file-exists-p file)
	         (ignore-errors
	           (with-temp-buffer
	             (insert-file-contents-literally file)
	             (goto-char (point-min))
	             (setq owner (read (current-buffer)))
	             (integerp owner)
	+            (process-attributes owner)
	+            (string= "emacs" (alist-get 'comm (process-attributes owner)))))
	         owner)))



In GNU Emacs 25.3.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.22.26)
 of 2017-12-16 built on build
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description:	Void Linux

Configured using:
 'configure --with-x-toolkit=gtk3 --with-xwidgets --prefix=/usr
 --sysconfdir=/etc --sbindir=/usr/bin --bindir=/usr/bin
 --mandir=/usr/share/man --infodir=/usr/share/info --localstatedir=/var
 --with-file-notification=inotify --with-modules --with-jpeg --with-tiff
 --with-gif --with-png --with-xpm --with-rsvg --without-imagemagick
 --with-xml2 --with-gnutls --with-sound --with-m17n-flt
 --host=x86_64-unknown-linux-gnu --build=x86_64-unknown-linux-gnu
 'CFLAGS=-fno-PIE -mtune=generic -O2 -pipe -g' 'CPPFLAGS= '
 'LDFLAGS=-no-pie -Wl,--as-needed ''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND DBUS GSETTINGS NOTIFY ACL GNUTLS
LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11
MODULES XWIDGETS

Important settings:
  value of $LC_COLLATE: C
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Date: Sun, 11 Feb 2018 10:54:13 +0100
> 
> 
> By default, desktop-save-mode will not save (and will set
> desktop-dirname to nil) if a lock file exists with a PID in it.
> 
> It's problematic however when Emacs gets killed and does not release the
> lock.  Upon next start, desktop-save-mode will refuse to save because
> the lock exists, even though no process is using it.  In terms of user
> experience, it's pretty bad considering the error feedback is just one
> line in the *Messages* buffer (I almost never notice it when it happens)
> and the problem is persistent across reboots (the lock file will remain
> as long as the user does not remove it manually).

When this happens to me, the next time I start Emacs, I'm asked
whether to use the desktop file that appears to be locked, and if I
answer YES, desktop.el goes on to restore the session, no other
questions asked, and saves the desktop upon exit with no problems.
I'm confused as to why you see something different.  Could it be that
your activation of desktop-save-mode is somehow different from mine?

> Furthermore, isn't it strange to just check if there lock file contains
> a number and not actually check if it's an existing PID?

You are assuming that the locking process runs on the same machine,
but that is not guaranteed.  The directory where the desktop file
lives could be mounted via the network, I think.

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 2271 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Pierre Neidhardt <ambrevar@gmail.com>
>> Date: Sun, 11 Feb 2018 10:54:13 +0100
>>
>>
>> By default, desktop-save-mode will not save (and will set
>> desktop-dirname to nil) if a lock file exists with a PID in it.
>>
>> It's problematic however when Emacs gets killed and does not release the
>> lock.  Upon next start, desktop-save-mode will refuse to save because
>> the lock exists, even though no process is using it.  In terms of user
>> experience, it's pretty bad considering the error feedback is just one
>> line in the *Messages* buffer (I almost never notice it when it happens)
>> and the problem is persistent across reboots (the lock file will remain
>> as long as the user does not remove it manually).
>
> When this happens to me, the next time I start Emacs, I'm asked
> whether to use the desktop file that appears to be locked, and if I
> answer YES, desktop.el goes on to restore the session, no other
> questions asked, and saves the desktop upon exit with no problems.
> I'm confused as to why you see something different.  Could it be that
> your activation of desktop-save-mode is somehow different from mine?

I always run Emacs daemon.  And I think the following excerpt explains
it all:

	(defun desktop-read (&optional dirname)
	...
	(or (null desktop-load-locked-desktop)
	    (daemonp)
	    (not (y-or-n-p (format "Warning: desktop file appears to be in use by PID %s.\n\
	Using it may cause conflicts.  Use it anyway? " owner))))

Not sure why (daemonp) is here.  Removing this line would solve the issue.

I just realized that `desktop-load-locked-desktop' is also a solution to
my problem.

>> Furthermore, isn't it strange to just check if there lock file contains
>> a number and not actually check if it's an existing PID?
>
> You are assuming that the locking process runs on the same machine,
> but that is not guaranteed.  The directory where the desktop file
> lives could be mounted via the network, I think.

You mean in case two Emacs instances on two separate machines share the
same location for `desktop-full-lock-name' over network?  I don't know,
is there such a use case?  I might not see the point.

--
Pierre Neidhardt

Troubles are like babies; they only grow by nursing.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Noam Postavsky]

Pierre Neidhardt <ambrevar@gmail.com> writes:

> I always run Emacs daemon.  And I think the following excerpt explains
> it all:
>
> 	(defun desktop-read (&optional dirname)
> 	...
> 	(or (null desktop-load-locked-desktop)
> 	    (daemonp)
> 	    (not (y-or-n-p (format "Warning: desktop file appears to be in use by PID %s.\n\
> 	Using it may cause conflicts.  Use it anyway? " owner))))
>
> Not sure why (daemonp) is here.  Removing this line would solve the issue.

'git blame' brings me to Bug#11674 "desktop doesn't handle unclean
restart under daemon mode".

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=11674

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  30421@debbugs.gnu.org
> Date: Sun, 11 Feb 2018 11:15:02 -0500
> 
> Pierre Neidhardt <ambrevar@gmail.com> writes:
> 
> > I always run Emacs daemon.  And I think the following excerpt explains
> > it all:
> >
> > 	(defun desktop-read (&optional dirname)
> > 	...
> > 	(or (null desktop-load-locked-desktop)
> > 	    (daemonp)
> > 	    (not (y-or-n-p (format "Warning: desktop file appears to be in use by PID %s.\n\
> > 	Using it may cause conflicts.  Use it anyway? " owner))))
> >
> > Not sure why (daemonp) is here.  Removing this line would solve the issue.
> 
> 'git blame' brings me to Bug#11674 "desktop doesn't handle unclean
> restart under daemon mode".
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=11674

Indeed: daemon cannot usefully ask this question.

So I think desktop-load-locked-desktop is indeed the right solution to
this situation.

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]


Setting `desktop-load-locked-desktop' works _only if_ I know there is no
other emacs (daemon or not) owning the lock.  We still have a conflict 
otherwise.

Hence my suggested patch.

It won't work in the scenario Eli mentioned, but I'm not sure how
relevant that is.  Isn't better than than generating conflict with local
instance of Emacs?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: Noam Postavsky <npostavs@users.sourceforge.net>, 30421@debbugs.gnu.org
> Date: Sun, 11 Feb 2018 17:57:29 +0100
> 
> Setting `desktop-load-locked-desktop' works _only if_ I know there is no
> other emacs (daemon or not) owning the lock.  We still have a conflict 
> otherwise.
> 
> Hence my suggested patch.
> 
> It won't work in the scenario Eli mentioned, but I'm not sure how
> relevant that is.  Isn't better than than generating conflict with local
> instance of Emacs?

No, I don't think so.

Why cannot you simply delete the lock file?  After all, a crashing
Emacs should be a somewhat rare phenomenon...

Alternatively, don't read desktop until the first client connects,
i.e. do that in a after-make-frame-functions hook.  Reading desktop in
daemon mode is problematic for other reasons as well: e.g., GUI frames
cannot be created and GUI features cannot be turned on.

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 479 bytes --]


> Alternatively, don't read desktop until the first client connects,
> i.e. do that in a after-make-frame-functions hook.  Reading desktop in
> daemon mode is problematic for other reasons as well: e.g., GUI frames
> cannot be created and GUI features cannot be turned on.

That's an excellent suggestions.  You are right, it has been troublesome
more than a few times.  I think both we should mention this in the
manual together with `desktop-load-locked-desktop'.

Thank you!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: npostavs@users.sourceforge.net, 30421@debbugs.gnu.org
> Date: Sun, 11 Feb 2018 18:23:49 +0100
> 
> > Alternatively, don't read desktop until the first client connects,
> > i.e. do that in a after-make-frame-functions hook.  Reading desktop in
> > daemon mode is problematic for other reasons as well: e.g., GUI frames
> > cannot be created and GUI features cannot be turned on.
> 
> That's an excellent suggestions.  You are right, it has been troublesome
> more than a few times.  I think both we should mention this in the
> manual together with `desktop-load-locked-desktop'.

Done.

> Thank you!

You are welcome.

is it okay to close this bug now?

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 308 bytes --]


One last question :) Is this the right way to initialize desktop-mode
after frames are made then?

	(defun desktop-init (_frame)
	  (desktop-save-mode)
	  (desktop-read)
	  0)
	(add-hook 'after-make-frame-functions 'desktop-init)

It seems that those functions are expected to return a number, hence the 0.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: npostavs@users.sourceforge.net, 30421@debbugs.gnu.org
> Date: Sun, 11 Feb 2018 19:41:22 +0100
> 
> One last question :) Is this the right way to initialize desktop-mode
> after frames are made then?
> 
> 	(defun desktop-init (_frame)
> 	  (desktop-save-mode)
> 	  (desktop-read)
> 	  0)
> 	(add-hook 'after-make-frame-functions 'desktop-init)

Yes, I think so.  Although you may wish doing this only once, not
after each frame creation (in which case, you should make the function
remove itself from the hook variable).

> It seems that those functions are expected to return a number, hence the 0.

Hmm... where did you see that they are expected to return a number?

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]


>> 	(defun desktop-init (_frame)
>> 	  (desktop-save-mode)
>> 	  (desktop-read)
>> 	  0)
>> 	(add-hook 'after-make-frame-functions 'desktop-init)
>
> Yes, I think so.  Although you may wish doing this only once, not
> after each frame creation (in which case, you should make the function
> remove itself from the hook variable).

Good point.

>> It seems that those functions are expected to return a number, hence the 0.
>
> Hmm... where did you see that they are expected to return a number?

Forget it, I had an error I cannot reproduce, must have been something else.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 681 bytes --]


Actually... Deferring the loading of desktop-mode to after-make-frame
cannot work (unless I'm missing the point):

		  if (and owner
			   (memq desktop-load-locked-desktop '(nil ask))
			   (or (null desktop-load-locked-desktop)
			       (daemonp)
			       (not (y-or-n-p (format "Warning: desktop file appears to be in use by PID %s.\n\ Using it may cause conflicts.  Use it anyway? " owner)))))

`(deamonp)` will still be true...

I could set `desktop-load-locked-desktop' to `t'
but then I'm assuming I'll always be the only desktop-mode client on
this machine... I don't like that.

What about changing `(daemonp)' to some predicate checking if there is a
frame conencted?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: npostavs@users.sourceforge.net, 30421@debbugs.gnu.org
> Date: Thu, 15 Feb 2018 23:56:17 +0100
> 
> 		  if (and owner
> 			   (memq desktop-load-locked-desktop '(nil ask))
> 			   (or (null desktop-load-locked-desktop)
> 			       (daemonp)
> 			       (not (y-or-n-p (format "Warning: desktop file appears to be in use by PID %s.\n\ Using it may cause conflicts.  Use it anyway? " owner)))))
> 
> `(deamonp)` will still be true...
> 
> I could set `desktop-load-locked-desktop' to `t'
> but then I'm assuming I'll always be the only desktop-mode client on
> this machine... I don't like that.
> 
> What about changing `(daemonp)' to some predicate checking if there is a
> frame conencted?

Something like that, but:

  . it has to be _in_addition_to_ daemonp check, i.e. the predicate
    should only be called in a daemon session
  . it should test for an existence of a client frames, not just _any_
    frames, because, perhaps surprisingly, a daemon session does have
    a frame, it just isn't displayed (and is not a GUI frame)

Patches to that effect are welcome.

Thanks.

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]


I tried patching with

                       -(daemonp)
                       +(and (daemonp) (= (length (visible-frame-list)) 1))

Then I killed the daemon, leaving a .lock behind.
On restart:

- In a tty, I get a black screen with a blinking cursor on the bottom left
corner.
If I press 'y', it proceeds with loading the desktop file.  I can verify
that from the *Messages* buffer.

- With EXWM, I get a black screen, but 'y' does not work.  So it's
effectively stuck.

I'm thinking that maybe `after-make-frame-functions' are run too early,
before the frame is fully ready.

Then I re-considered Eli's last remark: the daemon already has a frame, so I
thought maybe we should also guard the hook with a check on the frames count:

	(defun ambrevar/desktop-init (_frame)
	    (when (> (length (visible-frame-list)) 1)
	      (desktop-save-mode)
	      (desktop-read)
	      (remove-hook 'after-make-frame-functions 'ambrevar/desktop-init)))

That did not change anything.

Any clue?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: npostavs@users.sourceforge.net, 30421@debbugs.gnu.org
> Date: Fri, 16 Feb 2018 23:58:52 +0100
> 
> I tried patching with
> 
>                        -(daemonp)
>                        +(and (daemonp) (= (length (visible-frame-list)) 1))
> 
> Then I killed the daemon, leaving a .lock behind.
> On restart:
> 
> - In a tty, I get a black screen with a blinking cursor on the bottom left
> corner.
> If I press 'y', it proceeds with loading the desktop file.  I can verify
> that from the *Messages* buffer.
> 
> - With EXWM, I get a black screen, but 'y' does not work.  So it's
> effectively stuck.
> 
> I'm thinking that maybe `after-make-frame-functions' are run too early,
> before the frame is fully ready.
> 
> Then I re-considered Eli's last remark: the daemon already has a frame, so I
> thought maybe we should also guard the hook with a check on the frames count:
> 
> 	(defun ambrevar/desktop-init (_frame)
> 	    (when (> (length (visible-frame-list)) 1)
> 	      (desktop-save-mode)
> 	      (desktop-read)
> 	      (remove-hook 'after-make-frame-functions 'ambrevar/desktop-init)))
> 
> That did not change anything.
> 
> Any clue?

Instead of counting frames, I'd suggest using

  (frame-parameter FRAME 'client)

where FRAME is the argument with which your after-make-frame-functions
hook was called.  If this returns non-nil, FRAME is a frame created by
serving a request from emacsclient.

If this still doesn't work, please show the code of entire hook and
the complete recipe you used to test it, maybe there's something else
at work here.

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 6915 bytes --]


Did not work.  Recipe:

- init.el:

	(require 'desktop)
	
	(defun desktop-read (&optional dirname)
	  "Read and process the desktop file in directory DIRNAME.
	Look for a desktop file in DIRNAME, or if DIRNAME is omitted, look in
	directories listed in `desktop-path'.  If a desktop file is found, it
	is processed and `desktop-after-read-hook' is run.  If no desktop file
	is found, clear the desktop and run `desktop-no-desktop-file-hook'.
	This function is a no-op when Emacs is running in batch mode.
	It returns t if a desktop file was loaded, nil otherwise."
	  (interactive)
	  (unless noninteractive
	    (setq desktop-dirname
	          (file-name-as-directory
	           (expand-file-name
	            (or
	             ;; If DIRNAME is specified, use it.
	             (and (< 0 (length dirname)) dirname)
	             ;; Otherwise search desktop file in desktop-path.
	             (let ((dirs desktop-path))
	               (while (and dirs
	                           (not (file-exists-p
	                                 (desktop-full-file-name (car dirs)))))
	                 (setq dirs (cdr dirs)))
	               (and dirs (car dirs)))
	             ;; If not found and `desktop-path' is non-nil, use its first element.
	             (and desktop-path (car desktop-path))
	             ;; Default: .emacs.d.
	             user-emacs-directory))))
	    (if (file-exists-p (desktop-full-file-name))
	        ;; Desktop file found, but is it already in use?
	        (let ((desktop-first-buffer nil)
	              (desktop-buffer-ok-count 0)
	              (desktop-buffer-fail-count 0)
	              (owner (desktop-owner))
	              ;; Avoid desktop saving during evaluation of desktop buffer.
	              (desktop-save nil)
	              (desktop-autosave-was-enabled))
	          (if (and owner
	                   (memq desktop-load-locked-desktop '(nil ask))
	                   (or (null desktop-load-locked-desktop)
	                       ;; Without a visible frame, Emacs daemon cannot
	                       ;; prompt the user so we don't load.
	                       (and (daemonp) (= (length (visible-frame-list)) 1))
	                       (not (y-or-n-p (format "Warning: desktop file appears to be in use by PID %s.\n\
	Using it may cause conflicts.  Use it anyway? " owner)))))
	              (let ((default-directory desktop-dirname))
	                (setq desktop-dirname nil)
	                (run-hooks 'desktop-not-loaded-hook)
	                (unless desktop-dirname
	                  (message "Desktop file in use; not loaded.")))
	            (desktop-lazy-abort)
	            ;; Temporarily disable the autosave that will leave it
	            ;; disabled when loading the desktop fails with errors,
	            ;; thus not overwriting the desktop with broken contents.
	            (setq desktop-autosave-was-enabled
	                  (memq 'desktop-auto-save-set-timer (default-toplevel-value 'window-configuration-change-hook)))
	            (desktop-auto-save-disable)
	            ;; Evaluate desktop buffer and remember when it was modified.
	            (load (desktop-full-file-name) t t t)
	            (setq desktop-file-modtime (nth 5 (file-attributes (desktop-full-file-name))))
	            ;; If it wasn't already, mark it as in-use, to bother other
	            ;; desktop instances.
	            (unless (eq (emacs-pid) owner)
	              (condition-case nil
	                  (desktop-claim-lock)
	                (file-error (message "Couldn't record use of desktop file")
	                            (sit-for 1))))
	
	            (unless (desktop-restoring-frameset-p)
	              ;; `desktop-create-buffer' puts buffers at end of the buffer list.
	              ;; We want buffers existing prior to evaluating the desktop (and
	              ;; not reused) to be placed at the end of the buffer list, so we
	              ;; move them here.
	              (mapc 'bury-buffer
	                    (nreverse (cdr (memq desktop-first-buffer (nreverse (buffer-list))))))
	              (switch-to-buffer (car (buffer-list))))
	            (run-hooks 'desktop-delay-hook)
	            (setq desktop-delay-hook nil)
	            (desktop-restore-frameset)
	            (run-hooks 'desktop-after-read-hook)
	            (message "Desktop: %s%d buffer%s restored%s%s."
	                     (if desktop-saved-frameset
	                         (let ((fn (length (frameset-states desktop-saved-frameset))))
	                           (format "%d frame%s, "
	                                   fn (if (= fn 1) "" "s")))
	                       "")
	                     desktop-buffer-ok-count
	                     (if (= 1 desktop-buffer-ok-count) "" "s")
	                     (if (< 0 desktop-buffer-fail-count)
	                         (format ", %d failed to restore" desktop-buffer-fail-count)
	                       "")
	                     (if desktop-buffer-args-list
	                         (format ", %d to restore lazily"
	                                 (length desktop-buffer-args-list))
	                       ""))
	            (unless (desktop-restoring-frameset-p)
	              ;; Bury the *Messages* buffer to not reshow it when burying
	              ;; the buffer we switched to above.
	              (when (buffer-live-p (get-buffer "*Messages*"))
	                (bury-buffer "*Messages*"))
	              ;; Clear all windows' previous and next buffers, these have
	              ;; been corrupted by the `switch-to-buffer' calls in
	              ;; `desktop-restore-file-buffer' (bug#11556).  This is a
	              ;; brute force fix and should be replaced by a more subtle
	              ;; strategy eventually.
	              (walk-window-tree (lambda (window)
	                                  (set-window-prev-buffers window nil)
	                                  (set-window-next-buffers window nil))))
	            (setq desktop-saved-frameset nil)
	            (if desktop-autosave-was-enabled (desktop-auto-save-enable))
	            t))
	      ;; No desktop file found.
	      (let ((default-directory desktop-dirname))
	        (run-hooks 'desktop-no-desktop-file-hook))
	      (message "No desktop file.")
	      nil)))
	
	(when (daemonp)
	  (defun ambrevar/desktop-init (frame)
	    (when (frame-parameter frame 'client)
	      (desktop-save-mode)
	      (desktop-read)
	      (remove-hook 'after-make-frame-functions 'ambrevar/desktop-init)))
	  (add-hook 'after-make-frame-functions 'ambrevar/desktop-init))

- Leave a ~/.emacs.d/.emacs.desktop behind.
- Kill all emacs instances.
- `echo 0 > ~/.emacs.d/.emacs.desktop.lock`
- Optional: Switch to a TTY.
- Start `emacsclient -a '' -t` or `emacsclient -a '' -c` if not in a TTY.
- Emacs gives a blank screen (with a blinking cursor in a TTY). Press `n`.
- See the *Messages* buffer: the `n` was answering the desktop question.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: npostavs@users.sourceforge.net, 30421@debbugs.gnu.org
> Date: Sun, 18 Feb 2018 12:26:06 +0100
> 
> Did not work.  Recipe:

Thank you for your efforts.  I will look into this soon if no one
beats me to it.

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> Date: Sun, 18 Feb 2018 18:37:56 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 30421@debbugs.gnu.org, npostavs@users.sourceforge.net
> 
> > From: Pierre Neidhardt <ambrevar@gmail.com>
> > Cc: npostavs@users.sourceforge.net, 30421@debbugs.gnu.org
> > Date: Sun, 18 Feb 2018 12:26:06 +0100
> > 
> > Did not work.  Recipe:
> 
> Thank you for your efforts.  I will look into this soon if no one
> beats me to it.

If you invoke emacsclient with one or more file names, and then run
your code in a function that is in the server-switch-hook list, does
that work?  If so, I think we lack a hook, which should run after the
frame is displayed.  Asking a question in after-make-frame-functions
is too early, because the frame was not displayed yet at that time.

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 587 bytes --]


Correct me if I did not understand your instructions correctly.  I did this:

- Add this to my init.el

  (defun ambrevar/desktop-init ()
    (desktop-save-mode)
    (desktop-read)
    (remove-hook 'server-switch-hook 'ambrevar/desktop-init))
  (add-hook 'server-switch-hook 'ambrevar/desktop-init)

- Kill Emacs, leaving a .emacs.desktop.lock behind.

- Switch to a TTY.

- Start `emacs --daemon`.

- Run `emacsclient foo`.

I correctly get prompted if I want to load the desktop then.

Running `emacsclient -t` does not trigger the desktop loading, but I
guess that's to be expected.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: 30421@debbugs.gnu.org, npostavs@users.sourceforge.net
> Date: Sat, 24 Feb 2018 20:44:45 +0100
> 
> Correct me if I did not understand your instructions correctly.  I did this:
> 
> - Add this to my init.el
> 
>   (defun ambrevar/desktop-init ()
>     (desktop-save-mode)
>     (desktop-read)
>     (remove-hook 'server-switch-hook 'ambrevar/desktop-init))
>   (add-hook 'server-switch-hook 'ambrevar/desktop-init)
> 
> - Kill Emacs, leaving a .emacs.desktop.lock behind.
> 
> - Switch to a TTY.
> 
> - Start `emacs --daemon`.
> 
> - Run `emacsclient foo`.

Yes.

> I correctly get prompted if I want to load the desktop then.

OK, thanks for testing.

So like I said, we need a new hook, which would be called when the
server creates a frame, but doesn't switch to any new buffers in that
frame.

> Running `emacsclient -t` does not trigger the desktop loading, but I
> guess that's to be expected.

Right, because in that case server-switch-hook is not run.

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> Date: Sat, 24 Feb 2018 22:09:26 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 30421@debbugs.gnu.org, npostavs@users.sourceforge.net
> 
> > I correctly get prompted if I want to load the desktop then.
> 
> OK, thanks for testing.
> 
> So like I said, we need a new hook, which would be called when the
> server creates a frame, but doesn't switch to any new buffers in that
> frame.

Can you see if the patch below gives good results?  It should work
both when you invoke emacsclient with one or more file names, or
without any file names.  In both cases, the new hook
server-after-make-frame-hook should be invoked, with the new frame
being the selected frame.  If you run your code in that new hook, you
should get prompted whether you want to load the desktop, regardless
whether or not you specify files for emacscilent to visit.

diff --git a/lisp/server.el b/lisp/server.el
index a892203..ff03cbe 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -188,6 +188,13 @@ server-switch-hook
   :group 'server
   :type 'hook)
 
+(defcustom server-after-make-frame-hook nil
+  "Hook run when the Emacs server creates a client frame.
+The created frame is selected when the hook is called."
+  :group 'server
+  :type 'hook
+  :version "27.1")
+
 (defcustom server-done-hook nil
   "Hook run when done editing a buffer for the Emacs server."
   :group 'server
@@ -1336,9 +1343,11 @@ server-execute
            ((or isearch-mode (minibufferp))
             nil)
            ((and frame (null buffers))
+            (run-hooks 'server-after-make-frame-hook)
             (message "%s" (substitute-command-keys
                            "When done with this frame, type \\[delete-frame]")))
            ((not (null buffers))
+            (run-hooks 'server-after-make-frame-hook)
             (server-switch-buffer (car buffers) nil (cdr (car files)))
             (run-hooks 'server-switch-hook)
             (unless nowait

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]


Works perfectly thanks!

I think my code snippet or something similar would deserve an entry in
the desktop info section.

-- 
Pierre Neidhardt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: 30421@debbugs.gnu.org, npostavs@users.sourceforge.net
> Date: Sat, 03 Mar 2018 19:05:26 +0100
> 
> Works perfectly thanks!

Thanks, I pushed this to the master branch.

> I think my code snippet or something similar would deserve an entry in
> the desktop info section.

I wasn't sure how to do that.  Please feel free to review the
documentation changes I made and suggest where we should show code
snippets, and which snippets.

Thanks.

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1.1: Type: text/plain, Size: 18 bytes --]


What about this?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Add-example-of-delayed-desktop-loading.patch --]
[-- Type: text/x-patch, Size: 1055 bytes --]

From a0b72f3c6c38308deba5da1b30e53e9b1923c7e0 Mon Sep 17 00:00:00 2001
From: Pierre Neidhardt <ambrevar@gmail.com>
Date: Mon, 19 Mar 2018 16:32:55 +0530
Subject: [PATCH] Add example of delayed desktop loading

---
 doc/emacs/misc.texi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi
index 68bd308983..65f6f3d75e 100644
--- a/doc/emacs/misc.texi
+++ b/doc/emacs/misc.texi
@@ -2531,6 +2531,14 @@ Saving Emacs Sessions
 first client connects, by calling @code{desktop-read} in a hook
 function that you add to @code{server-after-make-frame-hook}
 (@pxref{Creating Frames,,, elisp, The Emacs Lisp Reference Manual}).
+A possible setup when @code{desktop-load-locked-desktop} is @code{nil}:
+@example
+(defun desktop-init (_frame)
+  (desktop-save-mode)
+  (desktop-read)
+  (remove-hook 'server-after-make-frame-hook 'desktop-init))
+(add-hook 'server-after-make-frame-hook 'desktop-init)
+@end example
 
 @node Recursive Edit
 @section Recursive Editing Levels
-- 
2.16.2


[-- Attachment #1.3: Type: text/plain, Size: 111 bytes --]


-- 
Pierre Neidhardt

Most people want either less corruption or more of a chance to
participate in it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: 30421-done@debbugs.gnu.org, npostavs@users.sourceforge.net
> Date: Mon, 19 Mar 2018 16:36:49 +0530
> 
> +A possible setup when @code{desktop-load-locked-desktop} is @code{nil}:

Did you really mean 'nil'?  I thought this was to allow Emacs to ask
the user, even in daemon mode, whether to load a locked desktop, no?

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 140 bytes --]


Indeed, I meant 'ask! :)

-- 
Pierre Neidhardt

You'll wish that you had done some of the hard things when they were easier
to do.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30421: 25.3; desktop.el: Steal lock when no living "emacs" process owns it

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > It's problematic however when Emacs gets killed and does not release the
  > lock.  Upon next start, desktop-save-mode will refuse to save because
  > the lock exists, even though no process is using it.

Does the same issue exist in VC?

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Skype: No way! See https://stallman.org/skype.html.