Re: [PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.

[Laurent Bercot]

Without this fix it will create zombies that might cause deadlocks


 No.

 On December 23rd, when you sent a message to the list describing your
issue, I sent a reply, the first reply you received, explaining what
was happening to you.
 Did you read it?
 Did ANYONE read it? Or is the thought of "the problem comes from
busybox init" too uncomfortable?

--
 Laurent

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.

[Ralf Friedl]

Markus Gothe wrote:
Can you run strace on a process invoked from the inittab? I surely 
have issues with doing that.


As far as killing the processes it doesnt matter which signal you use; 
processes will become zombified forever without the patch.
Yes, I can run strace on a process invoked from init. I can even run 
strace on PID 1.
Maybe there is something else wrong with your system, as your patch 
shouldn't have any effect on what happens when runsvdir is killed.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: [**EXTERNAL**] Re: Re: [PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.

[Cathey, Jim]

I concur.  If a process's parent dies, that process will NOT be a zombie, no 
way no how, unless either the system's init(8) process is broken, or there is 
some kind of bug in the kernel.  You do not need to reap your own children 
before dying, and you never have, not on any flavor of Unix since 1972 or 
thereabouts.

-- Jim

From: busybox [mailto:busybox-boun...@busybox.net] On Behalf Of Emmanuel Deloget
Sent: Wednesday, January 03, 2018 3:04 PM
To: Markus Gothe 
Cc: busybox 
Subject: [**EXTERNAL**] Re: Re: [PATCH] Fix runsvdir so it reaps children and 
avoid zombi processes when killed.

Hello,

On Wed, Jan 3, 2018 at 11:04 PM, Markus Gothe 
> wrote:
Can you run strace on a process invoked from the inittab? I surely have issues 
with doing that.

​This is definitely possible, but thou shall not forget -ff to follow forks 
(and prepare for an awfull lot of output, so maybe you should redirect that in 
some log file). ​



As far as killing the processes it doesnt matter which signal you use; 
processes will become zombified forever without the patch.


​I, too, don't get how runsvdir children can be zombified if runsvdir is 
killed. ​When runsvdir dies, the children are reparented to /bin/init which is, 
AFAIK, reaping them.

I won't disagree with you (you say you have zombies all over the place) but 
maybe the problem - and the right fix - lies somewhere else.

Best regards,

-- Emmanuel Deloget
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Re: [PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.

[Emmanuel Deloget]

Hello,

On Wed, Jan 3, 2018 at 11:04 PM, Markus Gothe 
wrote:

> Can you run strace on a process invoked from the inittab? I surely have
> issues with doing that.
>

​This is definitely possible, but thou shall not forget -ff to follow forks
(and prepare for an awfull lot of output, so maybe you should redirect that
in some log file). ​



>
> As far as killing the processes it doesnt matter which signal you use;
> processes will become zombified forever without the patch.
>


​I, too, don't get how runsvdir children can be zombified if runsvdir is
killed. ​When runsvdir dies, the children are reparented to /bin/init which
is, AFAIK, reaping them.

I won't disagree with you (you say you have zombies all over the place) but
maybe the problem - and the right fix - lies somewhere else.

Best regards,

-- Emmanuel Deloget
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] udhcpd: lease conflict

[Grant Edwards]

On 2018-01-03, Jiří Prchal  wrote:

> Could be, may be I'm absolutely wrong. But when should function
> "nobody_responds_to_arp" bee called, when ip is leased or not? And
> does it make sense to reserve ip for not responding mac?

Whether it "makes sense" could be debated, but that's how DHCP is
defined to work.  When a client request an IP address, the ack from
the server that grants an address contains a "lease time".  The client
owns that IP address until the lease time expires or until the client
sends an explicit release to the server -- the client does not lose
ownership of the IP because it's been rebooted or disconnected
temporarily.

IMO, that does make sense.  It prevents IP address churning as devices
are plugged/unplugged, rebooted, moved around, whatever.

If you want an address to become free shortly after a device is
disconnected from the network, then you need to set your lease time to
a small value.

-- 
Grant Edwards   grant.b.edwardsYow! Sign my PETITION.
  at   
  gmail.com

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

SV: Re: [PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.

[Markus Gothe]

  Can you run strace on a process invoked from the inittab? I surely have issues with doing that.As far as killing the processes it doesnt matter which signal you use; processes will become zombified forever without the patch.Skickad via BlackBerry Hub för Android   Från: ralf.fri...@online.deSkickat: 3 januari 2018 22:43Till: nietzs...@lysator.liu.se; busybox@busybox.netÄmne: Re: [PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.  Markus Gothe wrote:

  
  Without
this fix it will create zombies that might cause deadlocks, 
  especially
when respawned from the inittab and it dies because of a signal
(e.g. ‘killall -9 runsvdir').
  
  
  Installing
a simple SIGCHLD-handler makes the problem go away.
  
  
  Signed-off-by:
Markus Gothe 
  
  
  diff
--git a/runit/runsvdir.c b/runit/runsvdir.c
  index
11ab40a..dca2232 100644
  ---
a/runit/runsvdir.c
  +++
b/runit/runsvdir.c
  @@
-232,6 +232,15 @@ static NOINLINE int do_rescan(void)
   	return
need_rescan;
   }
   
  
  +static
void signal_child(int sig UNUSED_PARAM){
  +	/*
reap children if we die */
  +	for
(;;) {
  +		pid_t
pid = wait_any_nohang(NULL);
  +		if
(pid <= 0)
  +			break;
  +	}
  +}
  +
   int
runsvdir_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
   int
runsvdir_main(int argc UNUSED_PARAM, char **argv)
   {
  @@
-269,6 +278,11 @@ int runsvdir_main(int argc UNUSED_PARAM, char
**argv)
   		
*/
   		|
(i_am_init ? ((1 << SIGUSR1) | (1 << SIGUSR2) | (1
<< SIGINT)) : 0)
   		,
record_signo);
  +
  +	/*
Install child-handler for reaping children. */
  +	bb_signals(0
  +		+
(1 << SIGCHLD)
  +		,
signal_child);
   	svdir
= *argv++;
   
  
   #if
ENABLE_FEATURE_RUNSVDIR_LOG
  @@
-369,12 +383,14 @@ int runsvdir_main(int argc UNUSED_PARAM, char
**argv)
   #endif
   		{
   			unsigned
deadline = (need_rescan ? 1 : 5);
  +			sig_block(SIGCHLD);
   #if
ENABLE_FEATURE_RUNSVDIR_LOG
   			if
(rplog)
   poll(pfd,
1, deadline*1000);
   			else
   #endif
   sleep(deadline);
  +			sig_unblock(SIGCHLD);
   		}
   
  
   #if
ENABLE_FEATURE_RUNSVDIR_LOG

According to your previous email, the testcase is this: 'killall
runsvdir'.
So how does installing a handler for SIGCHLD have an effect on the
behavior of the program when it receives a SIGTERM? Why should there
be children some terminated children when runsvdir is killed?
And what exactly is the testcase besides 'killall runsvdir'? Can you
run strace on runsvdir and the child and show what happens? So far
you have proposed a patch, but it's not clear what problem it is
supposed to fix.
  

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 0/3] infra: make it possible to install without clobbering existing utilities

[Yann E. MORIN]

Hello All!

On 2017-12-28 23:49 +0100, Yann E. MORIN spake thusly:
> Even though there is a hint of code preventing installation of applets
> over existing utilities, this is flawed in two ways:
> 
>   - first, the non-clobbering conditional code does not account for
> installation as script wrappers,
> 
>   - second, only one option can be passed to the install script, which
> is not very useful.
> 
> And finally, there is no way to pass additional options to the install
> script when calling 'make install'.
> 
> This series fixes all three issues.

Any feedback on those three patches? ;-)

Regards,
Yann E. MORIN.

-- 
.-..--..
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN |  ___   |
| +33 223 225 172 `.---:  X  AGAINST  |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL|   v   conspiracy.  |
'--^---^--^'
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] udhcpd: lease conflict

[Denys Vlasenko]

On Wed, Jan 3, 2018 at 10:30 AM, Jiří Prchal  wrote:
>
>
> On 31.12.2017 10:32, Denys Vlasenko wrote:
>>
>> On Fri, Dec 15, 2017 at 1:41 PM, Jiri Prchal 
>> wrote:
>>>
>>> If there is lease for MAC which is no longer connected and only one IP in
>>> pool, it doesn't lease to new one mac until expires the old one.
>>
>>
>> This is how it is intended to work.
>
> Could be, may be I'm absolutely wrong. But when should function
> "nobody_responds_to_arp" bee called, when ip is leased or not?

In current code it is called in order to do an extra check that there
is really no machine on an IP about to be handed out.

> And does it
> make sense to reserve ip for not responding mac?

What if "not responding MAC" is a machine which is being rebooted,
or a machine which is perfectly fine but it is behind a switch which
is being rebooted at the moment?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.

[Ralf Friedl]

Markus Gothe wrote:

Without this fix it will create zombies that might cause deadlocks,
especially when respawned from the inittab and it dies because of a 
signal (e.g. ‘killall -9 runsvdir').


Installing a simple SIGCHLD-handler makes the problem go away.

Signed-off-by: Markus Gothe >


diff --git a/runit/runsvdir.c b/runit/runsvdir.c
index 11ab40a..dca2232 100644
--- a/runit/runsvdir.c
+++ b/runit/runsvdir.c
@@ -232,6 +232,15 @@ static NOINLINE int do_rescan(void)
return need_rescan;
 }


+static void signal_child(int sig UNUSED_PARAM){
+/* reap children if we die */
+for (;;) {
+pid_t pid = wait_any_nohang(NULL);
+if (pid <= 0)
+break;
+}
+}
+
 int runsvdir_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int runsvdir_main(int argc UNUSED_PARAM, char **argv)
 {
@@ -269,6 +278,11 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv)
*/
| (i_am_init ? ((1 << SIGUSR1) | (1 << SIGUSR2) | (1 << SIGINT)) : 0)
, record_signo);
+
+/* Install child-handler for reaping children. */
+bb_signals(0
++ (1 << SIGCHLD)
+, signal_child);
svdir = *argv++;


 #if ENABLE_FEATURE_RUNSVDIR_LOG
@@ -369,12 +383,14 @@ int runsvdir_main(int argc UNUSED_PARAM, char 
**argv)

 #endif
{
unsigned deadline = (need_rescan ? 1 : 5);
+sig_block(SIGCHLD);
 #if ENABLE_FEATURE_RUNSVDIR_LOG
if (rplog)
poll(pfd, 1, deadline*1000);
else
 #endif
sleep(deadline);
+sig_unblock(SIGCHLD);
}


 #if ENABLE_FEATURE_RUNSVDIR_LOG

According to your previous email, the testcase is this: 'killall runsvdir'.
So how does installing a handler for SIGCHLD have an effect on the 
behavior of the program when it receives a SIGTERM? Why should there be 
children some terminated children when runsvdir is killed?
And what exactly is the testcase besides 'killall runsvdir'? Can you run 
strace on runsvdir and the child and show what happens? So far you have 
proposed a patch, but it's not clear what problem it is supposed to fix.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.

[Markus Gothe]

Without this fix it will create zombies that might cause deadlocks, 
especially when respawned from the inittab and it dies because of a signal 
(e.g. ‘killall -9 runsvdir').

Installing a simple SIGCHLD-handler makes the problem go away.

Signed-off-by: Markus Gothe 

diff --git a/runit/runsvdir.c b/runit/runsvdir.c
index 11ab40a..dca2232 100644
--- a/runit/runsvdir.c
+++ b/runit/runsvdir.c
@@ -232,6 +232,15 @@ static NOINLINE int do_rescan(void)
return need_rescan;
 }
 
+static void signal_child(int sig UNUSED_PARAM){
+   /* reap children if we die */
+   for (;;) {
+   pid_t pid = wait_any_nohang(NULL);
+   if (pid <= 0)
+   break;
+   }
+}
+
 int runsvdir_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int runsvdir_main(int argc UNUSED_PARAM, char **argv)
 {
@@ -269,6 +278,11 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv)
 */
| (i_am_init ? ((1 << SIGUSR1) | (1 << SIGUSR2) | (1 << 
SIGINT)) : 0)
, record_signo);
+
+   /* Install child-handler for reaping children. */
+   bb_signals(0
+   + (1 << SIGCHLD)
+   , signal_child);
svdir = *argv++;
 
 #if ENABLE_FEATURE_RUNSVDIR_LOG
@@ -369,12 +383,14 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv)
 #endif
{
unsigned deadline = (need_rescan ? 1 : 5);
+   sig_block(SIGCHLD);
 #if ENABLE_FEATURE_RUNSVDIR_LOG
if (rplog)
poll(pfd, 1, deadline*1000);
else
 #endif
sleep(deadline);
+   sig_unblock(SIGCHLD);
}
 
 #if ENABLE_FEATURE_RUNSVDIR_LOG

--1.9.5 (Apple Git-50.3)--

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] udhcpd: lease conflict

[Jiří Prchal]

On 31.12.2017 10:32, Denys Vlasenko wrote:

On Fri, Dec 15, 2017 at 1:41 PM, Jiri Prchal  wrote:

If there is lease for MAC which is no longer connected and only one IP in pool, 
it doesn't lease to new one mac until expires the old one.


This is how it is intended to work.
Could be, may be I'm absolutely wrong. But when should function "nobody_responds_to_arp" bee called, when ip is leased 
or not? And does it make sense to reserve ip for not responding mac?



I think for this situation is there function "nobody_responds_to_arp", but it is not 
called if "lease" is true.
In patch I switched the logic and that function is called if is lease.

Signed-off-by: Jiri Prchal 
---
  networking/udhcp/dhcpd.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/networking/udhcp/dhcpd.c b/networking/udhcp/dhcpd.c
index 238542b..5eb0682 100644
--- a/networking/udhcp/dhcpd.c
+++ b/networking/udhcp/dhcpd.c
@@ -292,7 +292,7 @@ static uint32_t find_free_or_expired_nip(const uint8_t 
*safe_mac, unsigned arppi
 goto next_addr;

 lease = find_lease_by_nip(nip);
-   if (!lease) {
+   if (lease) {
  //TODO: DHCP servers do not always sit on the same subnet as clients: should 
*ping*, not arp-ping!
 if (nobody_responds_to_arp(nip, safe_mac, arpping_ms))
 return nip;
--
2.7.4




___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox