Re: [Chicken-hackers] [PATCH] Fix for #989 and hopefully #877 too

[Mario Domenech Goulart]

On Tue, 5 Nov 2013 16:52:16 +0100 Peter Bex <address@hidden> wrote:

> On Mon, Nov 04, 2013 at 10:09:54PM +0000, Mario Domenech Goulart wrote:
>> On Mon, 4 Nov 2013 21:09:06 +0100 Peter Bex <address@hidden> wrote:
>> 
>> > The patch's commit message explains it all :)  I hope it's clear enough
>> > to understand what's going on in the patch.  If not, feel free to ask
>> > me for clarification.  I hope I can provide it :/
>> >
>> > I've added a few comments to the code again to explain what's happening,
>> > as that was REALLY non-obvious (at least to me).  I've checked with
>> > Felix and his reply strengthened my belief in my understanding of this
>> > part of the GC's code.  Which is still not that strong yet :)
>> 
>> Excellent, Peter.  Thanks a lot.  I pushed you patched and have closed
>> #989.  Let's wait to see if #877 gets fixed too.
>
> As it turned out, the "sync" egg exposed a flaw in this fix.
> The scheduler actually overwrites ##sys#signal-hook using set!, which
> means the bookkeeping of "handling_interrupts" was wrong.  It would get
> set to 1, but when the scheduler got invoked through an interrupt of
> reason C_TIMER_INTERRUPT_NUMBER, it would call ##sys#schedule, which
> would possibly resume another thread, which surprisingly enough does
> not go via C_context_switch().  That meant the handling_interrupts would
> be left enabled, blocking any further timer interrupts from being
> received.  This meant the original thread in which the signal would
> be handled would never be invoked, so handling_interrupts would never
> be cleared.
>
> I've now reworked the ##sys#interrupt-hook code so it will pluck *all*
> pending interrupts from the list (the old one would stop whenever
> it encountered a pending interrupt with no handler).  This is done in
> a tight loop which keeps calling C_i_pending_interrupt() until there is
> none left.  This C function now sets handling_interrupts to 1 as soon
> as it plucks the first interrupt from the list, and only will set it
> to 0 when it has exhausted the pending interrupts list.  I've further
> simplified this by removing the "special" status of the first interrupt
> or any interrupts received in between - ie, the "interrupt_reason"
> variable.  This further simplifies the code in C_raise_interrupt()
> as well as C_i_pending_interrupt().  We don't need to care about the
> C_TIMER_INTERRUPT_NUMBER anymore, as it shouldn't appear in the
> interrupt vector anyway (but there *is* a slot for it...), so I've
> removed that check, which again further simplifies this stuff.
>
> The tests work, the sleep test from #989 works and the synch egg's tests
> no longer hang, so I think this patch should probably go in ASAP.

Thanks, Peter.  I could successfully run salmonella on the whole set of
linux eggs.  I pushed your patch.

Best wishes.
Mario
-- 
http://parenteses.org/mario