Re: Missing sentinel events

[Eli Zaretskii]

> From: Lars Ingebrigtsen <address@hidden>
> Date: Sun, 22 Jan 2017 18:09:40 +0100
> 
> The manual says that sentinels get codes like:
> 
> -----
> The string describing the event looks like one of the following:
> 
>     "finished\n".
>     "deleted\n".
>     "exited abnormally with code exitcode (core dumped)\n". The “core
>     dumped” part is optional, and only appears if the process dumped
>     core.
>     "failed with code fail-code\n".
>     [many more removed here]
> ---
> 
> But this is the code from process.c:
> 
>   else if (p->outfd < 0)
>     {
>       /* The counterparty may have closed the connection (especially
>        if the NSM prompt above take a long time), so recheck the file
>        descriptor here. */
>       pset_status (p, Qfailed);
>       deactivate_process (proc);
>     }
>   else if ((fd_callback_info[p->outfd].flags & NON_BLOCKING_CONNECT_FD) == 0)
>     {
>       /* If we cleared the connection wait mask before we did the TLS
>        setup, then we have to say that the process is finally "open"
>        here. */
>       pset_status (p, Qrun);
>       /* Execute the sentinel here.  If we had relied on status_notify
>        to do it later, it will read input from the process before
>        calling the sentinel.  */
>       exec_sentinel (proc, build_string ("open\n"));
>     }
> 
> I suspect that I just forgot to put those calls to exec_sentinel into
> the "fail" bits here and other places in the code when I rewrote these
> parts last February, like this:
> 
>   /* The DNS lookup failed. */
>   else if (connecting_status (p->status))
>     {
>       deactivate_process (proc);
>       pset_status (p, (list2
>                      (Qfailed,
>                       concat3 (build_string ("Name lookup of "),
>                                build_string (p->dns_request->ar_name),
>                                build_string (" failed")))));
>     }
> 
> Doesn't it seem like these should be calling the sentinel, too?  I think
> so, but there's a slight chance that adding more sentinel calls will
> break some user-level code that's not expecting to see any new
> messages...
> 
> And the manual says "looks like one of the following", and people may
> have interpreted that as an exhaustive list of event strings.

The list is exhaustive, so that interpretation is correct.

You are looking at the error symbol that is put into the process
status, but that's not what the sentinel will see.  It will see what
status_message will produce given the symbol.  So you really must look
at status_message and the strings it produces to see whether the list
in the manual is exhaustive or not.

> Any thoughts?  It's incomplete as it is now, so we should definitely add
> more sentinel calls, but should they all look like "failed with code
> fail-code\n" or should they be... more meaningful?

What more meaningful messages did you have in mind?  It's hard to
reason in the abstract.