Sorry for replying to my own message. Am 14.01.2016 um 22:18 schrieb 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 took much of my sleep tonight. So I'd rather like to take my wording back and escalate to being "seriously worried". After all: I have a tendency to be too terse on comments. But reading the code now I could easily be tempted to make changes causing another regression. I would not be surprised if an innocent reader would read the code with the comment missing and conclude that we save the local variable `stack_limit` and simply assign C_stack_limit to `saved_stack_limit`, which would bring the bug back. ((BTW: I've been hunting this one for years. First time the real reason escaped me thus I just worked around the symptom by limiting the mutation stack when it grew beyond bonds dues to missing gc. See C_mutation_stack_limit here: http://lists.nongnu.org/archive/html/chicken-hackers/2012-06/msg00051.html )) Apparently my comments where still too terse: even an experienced chicken hacker proof-reading the code did conclude: >>> It won't change anything in the normal, single-threaded case The old code did: interrupt_time = C_cpu_milliseconds(); pending_interrupts[ pending_interrupts_count++ ] = reason; However - please correct me if I'm wrong at that - C_cpu_milliseconds will result in yet another system call. Thus another signal may be dispatched at this point. And since `pending_interrupts_count` is still zero the first branch of the if statement would be executed again. This would not be as bad as in the multi threaded case, since it would not overwrite the stack limit with a value derived from another threads stack pointer, but it would still reduce the available stack size by another 1000 words. Thus after a long while the stack may shrink to zero. That however is exacerbated with the fix. Now this case would immediately set the saved_stack_size to stack_bottom and tie chicken into gc forever. That's why I swapped the assignments. > [....] > > Thus we probably need a memory barrier. Which one? > __sync_synchronize ? atomic_signal_fence ? __memory_barrier ? I'll probably aim at this the other day. Still I'd like to learn which memory barrier I should prefer. Any suggestions? Thanks /Jörg _______________________________________________ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers