Hi!

----

Attached (as "astksh20130814_sigqueuerepeat001.diff.txt") is a patch
which fixes a couple of problems with kill(1)'s |sigqueue()| support.

** Changes:
- kill(1) now has a --repeat option to repeat |sigqueue()| if it
returns with |EAGAIN|. It turns out this can happen *very* often if
the |SIGQUEUE_MAX| limit is reached. Since this happens very very
*OFTEN* (and the detection and re-try on shell level a bit cumbersome)
this option is "on" by default for -q/-Q but can be turned off via
--norepeat. If |sigqueue()| is repeated kill(1) calles |sched_yield()|
(see "Notes" below) to prevent the receiving process from being
"starved" away from the CPU. Note that this can never result in an
endless loop as signals are queued in-order by the sending process as
long as there is space in the queue. Priority inversion and other
issues are avoided by using |sched_yield()|.

- If --norepeat is active kill(1) will properly report |EAGAIN| as
error message. Previously it reported "No such process" in such cases
which was *infuriatingly* misleading (and send a couple of
Solaris+Linux people to a "wild goose chase" to check for kernel
bugs).

- kill(1) now has a -Q option to pass an address to |sigqueue()|

- kill(1) now has a --sendcont to control the sending of SIGCONT after
a signal was send (requested by Cedric... as it turns out the extra
SIGCONT may fill-up the signal queue twice as fast up to
|SIGQUEUE_MAX| limit if -q/-Q aren't used). The other issues were that
a user may wish to send stopped processes signals (to queue them) and
then resume the whole process group in one step. The old behaviour of
unconditionally resuming processes prevented that... --nosendcont now
allows that

- The logic for sending SIGCONT after sending a signal has been moved
to |b_kill()|

- -q/-Q are now only available if |sigqueue()| is supported. It
doesn't make much sense to provide -q/-Q if there is no way to send
the requested value. This has been confusing for at least one script
author who wasted a day of debugging with that mess


** Notes:
- |sched_yield()| is part of the same "POSIX realtime extenions"
package as |sigqueue()| and therefore should always be there when
|sigqueue()| is available


** Testing:
- Cedric, Olga and I have this patch in our trees since a ~two weeks
without problems

Comments/rants/feedback welcome... and please notify me if you change
the patch somehow...

----

Bye,
Roland

-- 
  __ .  . __
 (o.\ \/ /.o) [email protected]
  \__\/\/__/  MPEG specialist, C&&JAVA&&Sun&&Unix programmer
  /O /==\ O\  TEL +49 641 3992797
 (;O/ \/ \O;)
diff -r -u original/src/cmd/ksh93/bltins/trap.c 
build_clang_dgkfix/src/cmd/ksh93/bltins/trap.c
--- src/cmd/ksh93/bltins/trap.c 2013-08-09 21:59:35.000000000 +0200
+++ src/cmd/ksh93/bltins/trap.c 2013-08-18 23:57:46.696491954 +0200
@@ -35,7 +35,9 @@
 
 #define L_FLAG 1
 #define S_FLAG 2
+#define C_FLAG JOB_CFLAG
 #define Q_FLAG JOB_QFLAG
+#define R_FLAG JOB_RFLAG
 
 static int     sig_number(Shell_t*,const char*);
 
@@ -194,7 +196,7 @@
 int    b_kill(int argc,char *argv[],Shbltin_t *context)
 {
        register char *signame;
-       register int sig=SIGTERM, flag=0, n;
+       register int sig=SIGTERM, flag=C_FLAG|R_FLAG, n;
        register Shell_t *shp = context->shp;
        int usemenu = 0;
        NOT_USED(argc);
@@ -218,12 +220,27 @@
                case 'l':
                        flag |= L_FLAG;
                        break;
+#if _lib_sigqueue
                case 'q':
                        flag |= Q_FLAG;
-                       shp->sigval = opt_info.num;
+                       flag &= ~C_FLAG;
+                       memset(&shp->sigval, 0, sizeof(shp->sigval));
+                       shp->sigval.sival_int = opt_info.num;
+                       break;
+               case 'Q':
+                       flag |= Q_FLAG;
+                       flag &= ~C_FLAG;
+                       memset(&shp->sigval, 0, sizeof(shp->sigval));
+                       shp->sigval.sival_ptr = (void *)(((char 
*)0)+((ptrdiff_t)opt_info.num));
+                       break;
+               case 'R':
+                       opt_info.num?(flag |= R_FLAG):(flag &= ~R_FLAG);
+                       break;
+#endif
+               case 'C':
+                       opt_info.num?(flag |= C_FLAG):(flag &= ~C_FLAG);
                        break;
                case '?':
-                       shp->sigval = 0;
                        errormsg(SH_DICT,ERROR_usage(2), "%s", opt_info.arg);
                        break;
        }
@@ -233,7 +250,6 @@
                argv++;
        if(error_info.errors || flag==(L_FLAG|S_FLAG) || (!(*argv) && 
!(flag&L_FLAG)))
        {
-               shp->sigval = 0;
                errormsg(SH_DICT,ERROR_usage(2),"%s", optusage((char*)0));
        }
        /* just in case we send a kill -9 $$ */
@@ -251,7 +267,6 @@
                                if((sig=sig_number(shp,signame))<0)
                                {
                                        shp->exitval = 2;
-                                       shp->sigval = 0;
                                        
errormsg(SH_DICT,ERROR_exit(1),e_nosignal,signame);
                                }
                                sfprintf(sfstdout,"%d\n",sig);
@@ -267,9 +282,8 @@
                        errormsg(SH_DICT,ERROR_exit(1),e_nosignal,signame);
                }
        }
-       if(job_walk(shp,sfstdout,job_kill,sig|(flag&Q_FLAG),argv))
+       
if(job_walk(shp,sfstdout,job_kill,sig|(flag&(C_FLAG|Q_FLAG|R_FLAG)),argv))
                shp->exitval = 1;
-       shp->sigval = 0;
        return(shp->exitval);
 }
 
diff -r -u original/src/cmd/ksh93/data/builtins.c 
build_clang_dgkfix/src/cmd/ksh93/data/builtins.c
--- src/cmd/ksh93/data/builtins.c       2013-06-26 20:29:58.000000000 +0200
+++ src/cmd/ksh93/data/builtins.c       2013-08-19 00:18:07.910431667 +0200
@@ -1010,7 +1010,7 @@
 ;
 
 const char sh_optkill[]         = 
-"[-1c?\n@(#)$Id: kill (AT&T Research) 2012-07-05 $\n]"
+"[-1c?\n@(#)$Id: kill (AT&T Research) 2013-08-14 $\n]"
 USAGE_LICENSE
 "[+NAME?kill - terminate or signal process]"
 "[+DESCRIPTION?With the first form in which \b-l\b is not specified, "
@@ -1027,13 +1027,22 @@
        "due to a signal.  If a name is given the corresponding signal "
        "number will be written to standard output.  If a number is given "
        "the corresponding signal name will be written to standard output.]"
+"[C!:sendcont?Send SIGCONT to resume target process after sending the signal.]"
 "[l?List signal names or signal numbers rather than sending signals as "
        "described above.  "
        "The \b-n\b and \b-s\b options cannot be specified.]"
-"[q]#[n?On systems that support \asigqueue\a(2), send a queued signal with "
-       "message number \an\a.  The specified \ajob\as must be a positive "
-       "number.  On systems that do not support \asigqueue\a(2), a signal "
-       "is sent without the message number \an\a and will not be queued.]"
+#if _lib_sigqueue
+"[q]#[n?Queue a signal using \asigqueue\a(2) with the integer value \an\a."
+       "The specified \ajob\as must be a positive number. Implies "
+       "--nosendcont.]"
+"[Q]#[addr?Queue a signal using \asigqueue\a(2) with the address pointer "
+       "value \aaddr\a. Note that the value is only portable between "
+       "processes which have the same pointer size and endianess, "
+       "otherwise the result may be undefined."
+       "The specified \ajob\as must be a positive number. Implies "
+       "--nosendcont.]"
+"[R!:repeat?Repeat attempt to queue a signal if sigqueue() returns EAGAIN.]"
+#endif
 "[L?Same as \b-l\b except that of no argument is specified the signals will "
        "be listed in menu format as with select compound command.]"
 "[n]#[signum?Specify a signal number to send.  Signal numbers are not "
diff -r -u original/src/cmd/ksh93/data/msg.c 
build_clang_dgkfix/src/cmd/ksh93/data/msg.c
--- src/cmd/ksh93/data/msg.c    2013-05-23 17:25:11.000000000 +0200
+++ src/cmd/ksh93/data/msg.c    2013-08-18 21:29:01.630335807 +0200
@@ -83,6 +83,7 @@
 const char e_subscript[]       = "%s: subscript out of range";
 const char e_toodeep[]         = "%s: recursion too deep";
 const char e_access[]          = "permission denied";
+const char e_again[]           = "resource temporarily unavailable";
 #ifdef _cmd_universe
     const char e_nouniverse[]  = "universe not accessible";
 #endif /* _cmd_universe */
diff -r -u original/src/cmd/ksh93/features/sigfeatures 
build_clang_dgkfix/src/cmd/ksh93/features/sigfeatures
--- src/cmd/ksh93/features/sigfeatures  2013-04-06 00:58:27.000000000 +0200
+++ src/cmd/ksh93/features/sigfeatures  2013-08-19 00:15:42.440051841 +0200
@@ -1,11 +1,8 @@
-lib    sigblock,sigrelse,sigsetmask,sigprocmask,sigvec,sigqueue,sigaction
+lib    
sigblock,sigrelse,sigsetmask,sigprocmask,sigvec,sigqueue,sigaction,sched_yield
 typ    sigset_t                        ast.h signal.h
 mem    sigvec.sv_mask                  signal.h
 mem    siginfo_t.si_value.sigval_int   signal.h
 cat{
-       #ifndef _lib_sigqueue
-       #   define sigqueue(sig,action,val)     kill(sig,action)
-       #endif
        #ifndef _mem_sigvec_sv_mask
        #   undef _lib_sigvec
        #endif
diff -r -u original/src/cmd/ksh93/include/defs.h 
build_clang_dgkfix/src/cmd/ksh93/include/defs.h
--- src/cmd/ksh93/include/defs.h        2013-08-12 16:31:56.000000000 +0200
+++ src/cmd/ksh93/include/defs.h        2013-08-19 00:10:36.730731842 +0200
@@ -143,6 +143,12 @@
        Shwait_f        waitevent;
 };
 
+#if _lib_sigqueue
+typedef sigval_t sh_sigval_t;
+#else
+typedef int sh_sigval_t;
+#endif
+
 #define _SH_PRIVATE \
        struct shared   *gd;            /* global data */ \
        struct sh_scoped st;            /* scoped information */ \
@@ -230,7 +236,7 @@
        int             xargexit; \
        int             nenv; \
        int             lexsize; \
-       int             sigval; \
+       sh_sigval_t     sigval; \
        mode_t          mask; \
        Env_t           *env; \
        void            *init_context; \
diff -r -u original/src/cmd/ksh93/include/jobs.h 
build_clang_dgkfix/src/cmd/ksh93/include/jobs.h
--- src/cmd/ksh93/include/jobs.h        2013-07-08 20:43:18.000000000 +0200
+++ src/cmd/ksh93/include/jobs.h        2013-08-18 23:52:31.728297588 +0200
@@ -140,7 +140,9 @@
 #define JOB_NFLAG      2
 #define JOB_PFLAG      4
 #define JOB_NLFLAG     8
-#define JOB_QFLAG      0x100
+#define JOB_QFLAG      0x100 /* use |sigqueue()| */
+#define JOB_RFLAG      0x200 /* retry for |EAGAIN| */
+#define JOB_CFLAG      0x400 /* send SIGCONT */
 
 extern struct jobs job;
 
@@ -186,6 +188,7 @@
 extern const char      e_jobsrunning[];
 extern const char      e_nlspace[];
 extern const char      e_access[];
+extern const char      e_again[];
 extern const char      e_terminate[];
 extern const char      e_no_jctl[];
 extern const char      e_signo[];
diff -r -u original/src/cmd/ksh93/sh/jobs.c 
build_clang_dgkfix/src/cmd/ksh93/sh/jobs.c
--- src/cmd/ksh93/sh/jobs.c     2013-08-13 16:41:50.000000000 +0200
+++ src/cmd/ksh93/sh/jobs.c     2013-08-18 23:55:56.584886159 +0200
@@ -31,6 +31,10 @@
 
 #include       "defs.h"
 #include       <wait.h>
+#if _lib_sched_yield
+#include       <sched.h>
+#endif
+
 #include       "io.h"
 #include       "jobs.h"
 #include       "history.h"
@@ -1169,22 +1173,20 @@
        register pid_t pid;
        register int r;
        const char *msg;
-       int qflag = sig&JOB_QFLAG;
+#if _lib_sigqueue
+       bool qflag = (sig&JOB_QFLAG)?true:false;
+       bool rflag = (sig&JOB_RFLAG)?true:false;
+#endif
+       bool cflag = (sig&JOB_CFLAG)?true:false;
 #ifdef SIGTSTP
        bool stopsig;
 #endif
-#if _lib_sigqueue
-       union sigval sig_val;
-#else
-       int sig_val = 0;
-#endif
+
        if(pw==0)
                goto error;
        shp = pw->p_shp;
-#if _lib_sigqueue
-       sig_val.sival_int = shp->sigval;
-#endif
-       sig &= ~JOB_QFLAG;
+
+       sig &= ~(JOB_CFLAG|JOB_QFLAG|JOB_RFLAG);
 #ifdef SIGTSTP
        stopsig = (sig==SIGSTOP||sig==SIGTSTP||sig==SIGTTIN||sig==SIGTTOU);
 #else
@@ -1213,54 +1215,78 @@
                {
                        if(pid>=0)
                        {
+#if _lib_sigqueue
                                if(qflag)
                                {
                                        if(pid==0)
                                                goto no_sigqueue;
-                                       r = sigqueue(pid,sig,sig_val);
+                                       while (r = sigqueue(pid, sig, 
shp->sigval), rflag && (r < 0) && (errno == EAGAIN))
+                                       {
+#if _lib_sched_yield
+                                               /* Give other processes some 
CPU time */
+                                               (void)sched_yield();
+#endif
+                                               errno = 0;
+                                       }
                                }
                                else
+#endif
                                        r = kill(pid,sig);
                                if(r>=0 && !stopsig)
                                {
                                        if(pw->p_flag&P_STOPPED)
                                                pw->p_flag &= 
~(P_STOPPED|P_SIGNALLED);
-                                       if(sig && !qflag)
-                                               kill(pid,SIGCONT);
+                                       if(sig && cflag)
+                                               (void)kill(pid,SIGCONT);
                                }
                        }
                        else
                        {
+#if _lib_sigqueue
                                if(qflag)
                                        goto no_sigqueue;
+#endif
                                if((r = killpg(-pid,sig))>=0 && !stopsig)
                                {
                                        job_unstop(job_bypid(pw->p_pid));
-                                       if(sig)
-                                               killpg(-pid,SIGCONT);
+                                       if(sig && cflag)
+                                               (void)killpg(-pid,SIGCONT);
                                }
                        }
                }
 #else
                if(pid>=0)
                {
+#if _lib_sigqueue
                        if(qflag)
-                               r = sigqueue(pid,sig,sig_val);
+                               while (r = sigqueue(pid, sig, shp->sigval), 
rflag && (r < 0) && (errno == EAGAIN))
+                               {
+#if _lib_sched_yield
+                                       /* Give other processes some CPU time */
+                                       (void)sched_yield();
+#endif
+                                       errno = 0;
+                               }
                        else
+#endif
                                r = kill(pid,sig);
                }
                else
                {
+#if _lib_sigqueue
                        if(qflag)
                                goto no_sigqueue;
+#endif
                        r = killpg(-pid,sig);
                }
 #endif /* SIGTSTP */
        }
        else
        {
+#if _lib_sigqueue
                if(qflag)
                        goto no_sigqueue;
+#endif
                if(pid = pw->p_pgrp)
                {
                        r = killpg(pid,sig);
@@ -1285,22 +1311,34 @@
        if(r<0 && job_string)
        {
        error:
-               if(pw && by_number)
-                       msg = sh_translate(e_no_proc);
-               else
-                       msg = sh_translate(e_no_job);
-               if(errno == EPERM)
-                       msg = sh_translate(e_access);
-               sfprintf(sfstderr,"kill: %s: %s\n",job_string, msg);
+               switch(errno)
+               {
+                       case EPERM:
+                               msg = sh_translate(e_access);
+                               break;
+                       case EAGAIN:
+                               msg = sh_translate(e_again);
+                               break;
+                       default:
+                               if(pw && by_number)
+                                       msg = sh_translate(e_no_proc);
+                               else
+                                       msg = sh_translate(e_no_job);
+                               break;
+               }
+
+               sfprintf(sfstderr, "kill: %s: %s\n", job_string, msg);
                r = 2;
        }
        sh_delay(.001);
        job_unlock();
        return(r);
+#if _lib_sigqueue
 no_sigqueue:
        msg = sh_translate("queued signals require positive pids");
        sfprintf(sfstderr,"kill: %s: %s\n",job_string, msg);
        return(2);
+#endif
 }
 
 /*
_______________________________________________
ast-developers mailing list
[email protected]
http://lists.research.att.com/mailman/listinfo/ast-developers

Reply via email to