Re: SIGCHLD traps shouldn't recurse

[Oğuz]

On Tue, Apr 6, 2021 at 10:54 AM L A Walsh  wrote:

>
>
> On 2021/04/06 00:23, Oğuz wrote:
> > 5 Nisan 2021 Pazartesi tarihinde L A Walsh  > > yazdı:
> >
> > On 2021/04/03 00:41, Oğuz wrote:
> >
> >  but I
> > don't think it's useful at all because the number of pending
> > traps keeps
> > piling up, and there is no way to reset that number. If there
> > is no real
> > use case for recursive SIGCHLD traps (which I can't think of
> > any), I think
> > this should change; no SIGCHLD trap should be queued while a
> > SIGCHLD trap
> > is already in progress.
> >
> They have to be queued.  Else how do you process their ending?  It is
> something that take very little time.  The parent is given the chance
> to collect child accounting info and the child's status.  How long do
> you think that should take?  Sigchld handlers do bookkeeping, not
> significant computation.  Are you asking what happens if
> the application is misdesigned from the beginning to need
> more CPU than is available at a child's death?  If that happens then
> the application needs to be fixed.
>

I have no idea what you're talking about. See my first post, I demonstrated
the behavior that I think should be changed there.

Re: SIGCHLD traps shouldn't recurse

[L A Walsh]

On 2021/04/06 00:23, Oğuz wrote:
5 Nisan 2021 Pazartesi tarihinde L A Walsh > yazdı:


On 2021/04/03 00:41, Oğuz wrote:

 but I
don't think it's useful at all because the number of pending
traps keeps
piling up, and there is no way to reset that number. If there
is no real
use case for recursive SIGCHLD traps (which I can't think of
any), I think
this should change; no SIGCHLD trap should be queued while a
SIGCHLD trap
is already in progress.


They have to be queued.  Else how do you process their ending?  It is
something that take very little time.  The parent is given the chance
to collect child accounting info and the child's status.  How long do
you think that should take?  Sigchld handlers do bookkeeping, not
significant computation.  Are you asking what happens if
the application is misdesigned from the beginning to need
more CPU than is available at a child's death?  If that happens then
the application needs to be fixed.

Re: SIGCHLD traps shouldn't recurse

[Oğuz]

5 Nisan 2021 Pazartesi tarihinde L A Walsh  yazdı:

> On 2021/04/03 00:41, Oğuz wrote:
>
>>  but I
>> don't think it's useful at all because the number of pending traps keeps
>> piling up, and there is no way to reset that number. If there is no real
>> use case for recursive SIGCHLD traps (which I can't think of any), I think
>> this should change; no SIGCHLD trap should be queued while a SIGCHLD trap
>> is already in progress.
>>
>>
>  So what happens if a child dies while you are servicing the
> previous
> child?


That's a good question but I don't know the answer. What do you think
should happen? Plus, should SIGCHLD, when there is a trap set for it,
interrupt `wait' as other signals do?


>
> From the perl ipc page:
>
> sub REAPER {
>   my $child;
>   while (($waitedpid = waitpid(-1, WNOHANG)) > 0) {
>   logmsg "reaped $waitedpid" . ($? ? " with exit $?" : "");
>   }
>   $SIG{CHLD} = \ }
> ---
>The while loop grabs finished task stats before returning.
>
> With this change:
>>
>> diff --git a/trap.c b/trap.c
>> index dd1c9a56..5ce6ab4f 100644
>> --- a/trap.c
>> +++ b/trap.c
>> @@ -643,6 +643,8 @@ void
>>  queue_sigchld_trap (nchild)
>>   int nchild;
>>  {
>> +  if (sigmodes[SIGCHLD] & SIG_INPROGRESS)
>> +return;
>>if (nchild > 0)
>>  {
>>catch_flag = 1;
>>
>> bash behaves this way:
>>
>> $ trap uname chld
>> $ uname -sr
>> Linux 5.4.0-66-generic
>> Linux
>> $ uname -sr
>> Linux 5.4.0-66-generic
>> Linux
>> $
>>
>> and I think this is what average user would expect. Whether there's a
>> better fix is beyond me though.
>>
>>
> 
>Looks like your uname is called twice, whereas some langs (perl)
> tried to auto-cleanup such things.
>

`uname' is called once for each `uname -sr' there. With that stupid change
I suggested, a SIGCHLD from an asynchronous job is lost if it arrives while
the trap handler for SIGCHLD is running, so it's useless anyway. Man, what
a mess


-- 
Oğuz

Re: SIGCHLD traps shouldn't recurse

[L A Walsh]

On 2021/04/03 00:41, Oğuz wrote:

 but I
don't think it's useful at all because the number of pending traps keeps
piling up, and there is no way to reset that number. If there is no real
use case for recursive SIGCHLD traps (which I can't think of any), I think
this should change; no SIGCHLD trap should be queued while a SIGCHLD trap
is already in progress.
  
  
   So what happens if a child dies while you are servicing the previous

child?

From the perl ipc page:

sub REAPER {
  my $child;
  while (($waitedpid = waitpid(-1, WNOHANG)) > 0) {
  logmsg "reaped $waitedpid" . ($? ? " with exit $?" : "");
  }
  $SIG{CHLD} = \ }
---
   The while loop grabs finished task stats before returning.


With this change:

diff --git a/trap.c b/trap.c
index dd1c9a56..5ce6ab4f 100644
--- a/trap.c
+++ b/trap.c
@@ -643,6 +643,8 @@ void
 queue_sigchld_trap (nchild)
  int nchild;
 {
+  if (sigmodes[SIGCHLD] & SIG_INPROGRESS)
+return;
   if (nchild > 0)
 {
   catch_flag = 1;

bash behaves this way:

$ trap uname chld
$ uname -sr
Linux 5.4.0-66-generic
Linux
$ uname -sr
Linux 5.4.0-66-generic
Linux
$

and I think this is what average user would expect. Whether there's a
better fix is beyond me though.
  


   Looks like your uname is called twice, whereas some langs (perl)
tried to auto-cleanup such things.

SIGCHLD traps shouldn't recurse

[Oğuz]

See:

$ trap uname chld
$ uname -sr
Linux 5.4.0-66-generic
Linux
$ uname -sr
Linux 5.4.0-66-generic
Linux
Linux
$ uname -sr
Linux 5.4.0-66-generic
Linux
Linux
Linux
$
$ trap - chld
$ trap date chld
$ uname -sr
Linux 5.4.0-66-generic
Sat 03 Apr 2021 10:15:21 AM +03
Sat 03 Apr 2021 10:15:21 AM +03
Sat 03 Apr 2021 10:15:21 AM +03
Sat 03 Apr 2021 10:15:21 AM +03

Now, that a SIGCHLD trap is queued for each subshell including those
created while the trap handler is being executed kind of makes sense, but I
don't think it's useful at all because the number of pending traps keeps
piling up, and there is no way to reset that number. If there is no real
use case for recursive SIGCHLD traps (which I can't think of any), I think
this should change; no SIGCHLD trap should be queued while a SIGCHLD trap
is already in progress.

With this change:

diff --git a/trap.c b/trap.c
index dd1c9a56..5ce6ab4f 100644
--- a/trap.c
+++ b/trap.c
@@ -643,6 +643,8 @@ void
 queue_sigchld_trap (nchild)
  int nchild;
 {
+  if (sigmodes[SIGCHLD] & SIG_INPROGRESS)
+return;
   if (nchild > 0)
 {
   catch_flag = 1;

bash behaves this way:

$ trap uname chld
$ uname -sr
Linux 5.4.0-66-generic
Linux
$ uname -sr
Linux 5.4.0-66-generic
Linux
$

and I think this is what average user would expect. Whether there's a
better fix is beyond me though.