Re: [Chicken-hackers] PATCH: allow signal handlers to be called from any thread.

[Jörg F . Wittenberger]

Great that this made it so far.

Am 14.01.2016 um 20:20 schrieb Peter Bex:
> On Fri, Jan 15, 2016 at 12:59:28AM +1300, Evan Hanson wrote:
>> Hi all,
>>
>> Attached is a signed-off copy of this patch, with some of the info in
>> the comments moved into the commit message.

Actually I'm slightly worried that those comments are stashed away
somewhere in the commit message.  The issue is somewhat thorny (as you
can see from the number of failed attempts I made on it).

>> It won't change anything in
>> the normal, single-threaded case, and it can only improve behaviour in
>> multi-threaded programs, so it's fine with me.

"only improve" is what it actually does, sadly...

at least it will still loose signals (as I noted before)

>> Thanks, Jörg, for investigating this issue. The report and especially
>> patch are very much appreciated.
> 
> Thanks to you both; pushed.  I like how this simplifies the code a bit
> and still improves the reliability of signal handling.

I tried to strike a balance between doing the right thing (and produce a
huge unintelligible overkill) and getting something acceptable done.

And I must admit that I don't know enough about recent C specs and how
to do this correctly in the presence of multiple threads running on
multiple cores each handling yet another signal dispatch.

There is still some chance of of a second thread seeing
   "pending_interrupts_count == 0 && !handling_interrupts"
to be true.

However if the other core had already assigned
C_stack_limit=stack_bottom we still smash saved_stack_limit !

Thus we probably need a memory barrier.  Which one?
__sync_synchronize ? atomic_signal_fence ? __memory_barrier ?

shouldn't we at least swap the terms in the `if` to
 "!handling_interrupts && pending_interrupts_count == 0"
and further have a local variable like `this_int_count` assigned like
this instead:
 "!handling_interrupts && (this_int_count=pending_interrupts_count++) == 0"

plus - sure - replaced the next line
 "pending_interrupts[ pending_interrupts_count++ ] = reason;"
with
 "pending_interrupts[ this_int_count ] = reason;"

???

> 
> Cheers,
> Peter
> 
> 
> 
> _______________________________________________
> Chicken-hackers mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/chicken-hackers
>