Bug 623460

Summary: rcxdm restart doesn't wait for stop before issuing start
Product: [openSUSE] openSUSE 11.3 Reporter: Jon Nelson <jnelson-suse>
Component: X.OrgAssignee: Dr. Werner Fink <werner>
Status: RESOLVED FIXED QA Contact: E-mail List <xorg-maintainer-bugs>
Severity: Normal    
Priority: P3 - Medium CC: coolo, mt, ro, werner
Version: Final   
Target Milestone: ---   
Hardware: All   
OS: openSUSE 11.3   
Whiteboard: .
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Bug Depends on: 622058    
Bug Blocks:    

Description Jon Nelson 2010-07-19 13:35:03 UTC
+++ This bug was initially created as a clone of Bug #622058 +++

the xdm init script suffers from the same issue discovered in Bug 622058.
Can a fix for the xdm script also be supplied?
Actually, this seems like it might be a more systemic issue. Why was the change made in the first place?  It seems like this issue might effect many packages.
Comment 1 Stefan Dirsch 2010-07-19 13:50:38 UTC
> Why was the change made in the first place?

Which change?
Comment 2 Jon Nelson 2010-07-19 14:19:31 UTC
As noted in bug 622068 (comment #5), the behavior of killproc was changed to meet LSB specs (or something like that). The upshot is that _prior_ to 11.3, when invoked like this:

killproc -TERM /some/process/path

it would send a -TERM.  wait (normally) 5 seconds and if the process still hasn't exited, send a SIGKILL.  The new behavior eliminates the wait and subsequent SIGKILL.

Thus, what happens _now_, in very rapid succession:

obtain pid of (for example) /usr/bin/kdm
kill -TERM $that_pid
start /usr/bin/kdm

The problem is that the old kdm hasn't exited yet (the signal was sent just one one-jillionth of a second ago, after all).

Thus, "rcxdm restart" frequently only ends up killing the current display manager, as it can't start the new one.


This change in the behavior of killproc seems terribly ill-advised without also having had a good look at **all** of the init scripts which use it.


In any case, the change in bug 622058 should, IMO, be applied here as well:

Use:

    killproc ${PIDFILE:+-p ${PIDFILE}} -t 10 -TERM $DISPLAYMANAGER

instead of:

    killproc ${PIDFILE:+-p ${PIDFILE}} -TERM $DISPLAYMANAGER

Indeed, the comment immediately prior to the killproc code reads:

    # 
    # killproc(8) sleep upto five seconds and sends
    # SIGKILL if xdm does not terminate within
    #

and that comment is now *wrong*.
Comment 3 Dr. Werner Fink 2010-07-19 14:41:31 UTC
Remove the -TERM as LSB requires that if the signal is specified
the killproc should only send the signal specified on the command
line ... if no signal is specified on the command line the
signal TERM is used followed by KILL after 5 seconds.
Comment 4 Jon Nelson 2010-07-19 14:49:19 UTC
Dr. Werner Fink - that seems like a more reasonable solution.
If I may ask - did anybody bother to go through the init scripts to look for applications which depend on the (old) behavior of killproc?

Just a suggestion: perhaps someone should to head off more bugs.
Comment 5 Dr. Werner Fink 2010-07-19 14:57:24 UTC
Normally the package should not depend on the old behaviour that is
that 5 seconds should be enough to terminate a normal server daemon.

And yes I've look through out the services.   Currently only cups
is known to have a problem for restart.

The question is *why* SIGTERM takes that long for xdm od gdm or xdm
or any other display manager?  And why was this problem discoverd that
late as the changes was done at Mon Apr 12 17:49:46 CEST 2010.

Compare with bug #595796 and bug #622058
Comment 6 Stefan Dirsch 2010-07-19 15:00:38 UTC
Werner? I wasn't aware of that change at all ...
Comment 7 Dr. Werner Fink 2010-07-19 15:16:38 UTC
See request 43403 and request 43404
Comment 8 Stefan Dirsch 2010-07-19 15:37:24 UTC
Ok. Accepter for X11:XOrg and submitted to openSUSE:Factory.
Comment 9 Jon Nelson 2010-07-19 16:11:44 UTC
(In reply to comment #5)
> Normally the package should not depend on the old behaviour that is
> that 5 seconds should be enough to terminate a normal server daemon.
> 
> And yes I've look through out the services.   Currently only cups
> is known to have a problem for restart.
> 
> The question is *why* SIGTERM takes that long for xdm od gdm or xdm
> or any other display manager?  And why was this problem discoverd that
> late as the changes was done at Mon Apr 12 17:49:46 CEST 2010.
> 
> Compare with bug #595796 and bug #622058

With all due respect, I believe that there still exists an issue with lots of packages.  A quick "grep TERM /etc/init.d/* | grep killproc" shows 36 of 110 init scripts (on this machine) make use of killproc -TERM in some way.

I recently discovered that haldaemon is *also* susceptible to this particular issue.

To me, the core issue is the fact that many init scripts are specifying the -TERM which means that with the new killproc behavior they do not wait _at all_.  

I would wager that a great many of the init scripts which currently specify -TERM would not reliably restart their packages if the system were under any sort of load or the program took more than a fraction of a second to exit.
Comment 10 Dr. Werner Fink 2010-07-20 09:54:38 UTC
I've submitted a changed version of killproc (package sysvinit).
It now waits upto 5 seconds even if SIGTERM was specified on the command
line and therefore the final SIGKILL is not used as required for LSB.

See request https://build.opensuse.org/request/show/43484
and https://build.opensuse.org/request/show/43483 .
Comment 11 Dr. Werner Fink 2010-07-22 10:43:37 UTC
Reopen for SWAMID
Comment 12 Dr. Werner Fink 2010-07-22 10:44:56 UTC
A SWAMPID for sysvinit and xorg-x11 for OS11.3 is required.
Comment 13 Marcus Meissner 2010-07-22 13:30:57 UTC
sounds reasonible. although the X update might be quite large ... 
can we postpone that part and collect other X fixes first?

sysvinit can be done now I guess +1
Comment 14 Stefan Dirsch 2010-07-22 14:41:26 UTC
Actually I'm not really interested into an update of xorg-x11 for 11.3 for that issue at all. I don't believe it's that important.
Comment 15 Swamp Workflow Management 2010-07-22 15:05:07 UTC
The SWAMPID for this issue is 34705.
This issue was rated as low.
Please submit fixed packages as soon as possible.
Also create a patchinfo file using this link:
https://swamp.suse.de/webswamp/wf/34705
Comment 16 Christian Dengler 2010-07-22 15:05:48 UTC
Ok, update process started for sysvinit. For xorg we can collect more bugs and start an update then.
Comment 17 Dr. Werner Fink 2010-07-23 07:57:22 UTC
Patchinfo submitted