Re: [PATCH] A myriad of interrelated daemon-mode startup bugs

[Nix]

On 13 Nov 2012, Stefan Monnier stated:

[Sorry for the delay. Holidays, illness, prevarication, you know how it
 is...]

>>>> -      (cl-letf (((default-file-modes) ?\700)) (make-directory dir t))
>>>> +      (make-directory dir t ?\700)
>>> The cl-letf ends up doing the exact same thing, via dynamic scoping
>>> (note how it's not a simple `let' and it doesn't bind a variable but
>>> a "place").
>> Ah! Mind you, there's not much hint in the docs for cl-letf that it's
>> dynamically bound...
>
> lexical-scoping for non-variables is pretty hard to even define, so if
> you think hard about it (and follow the consequence of "Each PLACE is
> set to the corresponding VALUE, then the BODY forms are executed.
> On exit, the PLACEs are set back to their original values") you'll see
> it can only be dynamically scoped.

Agreed -- but I think that perhaps a slightly more explicit statement
would be a good idea. (It takes a bit of thought even to realise that
PLACEs are not a sort of variable. Certainly I'd never thought of it
that way before now.)

>>> I'm not completely opposed to adding a "modes" argument to
>>> make-directory, but I'm not sure it's worth the trouble.
>> It's a potential security hole to make directories with the wrong mode,
>> so it seems like a good idea. (Sure, you can bind `default-file-modes',
>> but could you really call the code above particularly clear?)
>
> I'm a firm believer in explicit arguments and lexical-scoping, but
> it has to be weighed against the cost of change.  As written above, I'm
> not sure which side wins here.

I'm just being lazy and copying the Unix API: mkdir() et al have
explicit modes as well as a umask, so why don't we?

>>> (unwind-protect
>>> (server-start)
>>> (if server-process
>>> (daemon-initialized)
>>> (if (stringp dn)
>>> ...
>
>> That works, but it throws away the error message.  If `server-start'
>> raised an error, you want to show it to the user before you die.
>
> No, unwind-protect doesn't catch the error and doesn't silence it either.

True. Sorry, definite thinko there :) if unwind-protect silenced errors,
it'd be next to useless.

>>>> +  ;; We must pretend to be noninteractive at this point to tell
>>>> +  ;; kill-emacs-hook functions that read input if interactive
>>>> +  ;; not to do so, since reads from stdin are shared with the
>>>> +  ;; parent.
>>>> +  (let ((noninteractive t))
>>>> +      (kill-emacs 1)))))
>>> Actually, `noninteractive' can sometimes interact with the user via
>>> stdin/stdout.
>> OK, so that ugly kludge is presumably unnecessary? Phew.
>
> No, but when the kill-emacs-hook code uses `noninteractive' it may do it
> to check something else than that you intended.

True.

>>> How 'bout having a Lisp-exported variable that controls whether to call
>>> kill-emacs here?  Then server.el could set this var (after successfully
>>> starting the daemon) to prevent exit.
>> Instead of daemon-initialized? The worry there is that server-start can
>> be called from places *other* than daemonization, and it seems like a
>> layering violation to have it saying 'yeah, you can exit now' if it was
>> called from somewhere else.
>
> Hmm... you mean that my Emacs wouldn't die any more upon C-x C-c because
> my .emacs calls server-start.  Right.  Hmm...

Your .emacs, or just someone by hand (heck, it's interactive). I don't
thik we really want to have someone doing an M-x server-start to find
that it kills their Emacs stone dead if it fails! :}

>>> Also, maybe the right way to solve this is to think harder about what
>>> noninteractive means and how to split it into several variables.
>>> E.g. one meaning is "no input events", another is "I/O is via
>>> stdin/stdout", and another we need is "there's no I/O available right
>>> now".
>> That's exactly what I was thinking while I got repeatedly confused over
>> just what 'noninteractive' meant right now. At the moment, it seems to
>> be used to mean whichever of the above is most convenient at the time :)
>
> Yes, it's pretty messy.  The original meaning was "running in -batch
> mode", i.e. with I/O via the special terminal that's bound to
> stdin/stdout.  So maybe we should just add another var for "no I/O
> available" (e.g. after closing all terminals, either about to exit or
> waiting for new emacsclient connections).  Or rather a predicate (which
> just checks the list of open terminals and decides whether there's one
> that can offer I/O).

Quite. Even the patch I submitted wasn't enough, btw -- I got another
waiting-for-input-with-no-input-available freeze. It seems sensible that
the input loop should simply not, well, loop unless there is somewhere
it could get input from.

So, I'll work on that approach.

-- 
NULL && (void)