Re: [PATCH] Do not send duplicate signals when MONITOR is set

[Lawrence Velázquez <larryv@xxxxxxx>]

On Mon, Jun 14, 2021, at 3:19 PM, Peter Stephenson wrote:
> On Mon, 2021-06-07 at 11:45 -0700, Bart Schaefer wrote:
> > On Mon, Jun 7, 2021 at 10:28 AM Erik Paulson <epaulson10@xxxxxxxxx> wrote:
> > > 
> > > I run emacs as a daemon and use the emacsclient program to connect to
> > > it. I noticed that when I suspended the emacsclient program and
> > > resumed it in zsh, the program would sporadically crash. After digging
> > > into the code, I realized that emacsclient was receiving two SIGCONTs,
> > > which caused it to send a malformed command to the daemon.
> > > 
> > > I found that this return used to be present, but was removed in
> > > https://www.zsh.org/mla/workers/2018/msg01338.html while addressing
> > > another emacs issue.
> > 
> > I don't think it was removed ... similar code was added in two
> > separate places, but the "return" was only added in one of those.
> > 
> > Your patch adds that return in the second case.
> > 
> > The difference is that in the first case, the SIGCONT is received by a
> > job that is marked STAT_SUPERJOB and in the second case it's received
> > by a different job.
> > 
> > I believe this means that in the former case the superjob is in the
> > foreground and in the second case, it isn't -- rather one of its
> > subjobs is.  In the first instance zsh sends the signal to all the
> > subjobs and then to the process group.  In the second case it sends
> > the signal to the process group first and then falls into the loop
> > sending the signal to any subjobs that still appear to be stopped.
> > 
> > In any case I think a potential problem with placing an unconditional
> > "return" where your patch does, is that signals other than SIGCONT
> > probably still need to be delivered to the subjobs.  PWS, any input
> > here?
> 
> Hmm, it's not clear to me in what cases you'd need to deliver both
> killpg() to the group leader and then kill to the processes.  You might
> hope if the shell was doing the right thing the first would be enough.
> But there might be special cases.
> 
> I would hazard that as SIGCONT is probably the most difficult case ---
> the only one where you specifically want the process to be running
> afterwards --- if this patch improves things there, it's prohably not
> doing a lot of harm in most cases.
> 
> It's not impossible there are some oddities where process groups aren't
> set up quite how you want that this might affect, but I doubt we're
> going to spot them just by staring.

Anything else on this?

-- 
vq