On 12 June 2013 20:11, Cedric Blancher <[email protected]> wrote:
> On 12 June 2013 19:59, Cedric Blancher <[email protected]> wrote:
>> On 12 June 2013 18:56, Cedric Blancher <[email protected]>
>> wrote:
>>> On 12 June 2013 13:46, Glenn Fowler <[email protected]> wrote:
>>>> the ast-ksh 2013-06-11 alpha source has been posted to
>>>> http://www.research.att.com/sw/download/alpha/
>>>> its still a work in progress
>>>> dgk and I are corrdinating the new vmalloc implementation vs signals
>>>> in addition to the work dgk is doing in ksh vs signals
>>>
>>> signals still do NOT work. For example my test below should print (
>>> typeset -l -E i=200.00200 ) but as you can see below it doesn't sum
>>> up:
>>> ksh -c 'compound a=(float i=0) ; trap "((a.i+=.00001));kill -USR2 $$&
>>> :" USR1 ; trap "((a.i+=1))" USR2 ; for ((j=0;j<200;j++));do kill -USR1
>>> $$&;done ; true ; while ! wait ; do true;done;print ${a}'
>>> ( typeset -l -E i=58.00094 )
>>> ksh -c 'compound a=(float i=0) ; trap "((a.i+=.00001));kill -USR2 $$&
>>> :" USR1 ; trap "((a.i+=1))" USR2 ; for ((j=0;j<200;j++));do kill -USR1
>>> $$&;done ; true ; while ! wait ; do true;done;print ${a}'
>>> ( typeset -l -E i=24.00048 )
>>
>> I noticed some problems in the code:
>> +#ifdef _lib_sigaction
>> + static void set_trapinfo(Shell_t *shp, int sig, siginfo_t *info)
>> + {
>> + sigset_t set,oset;
>> + if(!shp->siginfo)
>> + shp->siginfo = (void**)calloc(sizeof(void*),shp->gd->sigmax);
>>
>> This should IMO not be done in the signal handler. if multiple signal
>> handlers are running stacked on each other it may cause a race
>> condition between the if(!shp->siginfo) and the value assignment to
>> shp->siginfo.Actually the timing window is quite huge.
>>
>> + if(info)
>> + {
>> + struct Siginfo *jp,*ip;
>> + sigfillset(&set);
>> + sigprocmask(SIG_BLOCK,&set,&oset);
>> + ip = malloc(sizeof(struct Siginfo));
>> + sigprocmask(SIG_UNBLOCK,&oset,&set);
>> + ip->next = 0;
>> + memcpy(&ip->info,info,sizeof(siginfo_t));
>> + if(!(jp=(struct Siginfo*)shp->siginfo[sig]))
>>
>> Isn't SIG_BLOCK a good way to loose signals? What happens if new
>> signals arrive while the signal block is active? Are they queued or
>> discarded? I think they are discarded and the time spend in the
>> allocator gives another large window of opportunity to screw up and
>> loose more signals.
>
> +#ifdef SIGRTMIN
> + register int flag, sig=SIGRTMIN;
> +#else
> register int flag, sig=shp->st.trapmax;
> +#endif
>
> Why is sig set to SIGRTMIN? Doesn't this prevent signals > SIGRTMIN,
> i.e. SIGRTMAX from being processed? there's no POSIX gurantee that the
> realtime signals are on the top of the signal list either.
>
> while(sig-- > 0)
> {
> if(trap=shp->st.trapcom[sig])
>
> The while loop to count down the signal numbers has to be removed too.
> Otherwise signals are not processed by traps in the order they've
> arrived.
>
> + struct Siginfo *ip=0, *ipnext;
> +retry:
> + sighold(sig);
> + if(shp->siginfo)
> + {
> + ip = (struct
> Siginfo*)shp->siginfo[sig];
> + shp->siginfo[sig] = 0;
> + }
> + again:
>
> What is the purpose of sighold()? Why is sighold() required when the
> signals are queued in sh_fault() running on the signal handler stack
> and processed on the user stack?
The use of sighold() may be the culprit for some issues. The Linux
manpage clearly says " This API is obsolete: new appli-
cations should use the POSIX signal API (sigaction(2),
sigprocmask(2), etc.)". On some platforms its even not possible to
safely mix sighold() with sigaction() without loosing signals.
The other issue which jumped in my eye is that the queue management is
far from async signal safe and that the natural ordering of signals is
badly disturbed by processing them by signal number instead of the
order they've been received.
Did anyone thought about the idea of using signalfd() instead?
Lionel
_______________________________________________
ast-developers mailing list
[email protected]
http://lists.research.att.com/mailman/listinfo/ast-developers
