Re: [Chicken-hackers] Scheduler fdset assert patch

[Jonathan Chan]

I realized that if I am able to set a flag about whether or not we are
in the interrupt hook, I might as well save the previous block object of
the thread and then restore it instead of possibly causing spurious
wakeups. As an aside, it seems as if force-primordial doesn't currently
do this, and relies on the code that asked the scheduler to block on i/o
to retry on its own. As I stated in the previous email, this seems to be
what the code that uses the scheduler's blocking facilities does anyway.

In any case, this patch also passes my tests (as the previous one did)
but now should more cleanly avoid "spurious wakeups" of threads blocking
on I/O.

Also, I realized that the test case lacked instructions; you should just
be able to start it, wait a second, then send SIGINT (e.g. with Ctrl-C)
and the unpatched scheduler should cause the program to exit with an
assert error while with the patched scheduler you should just see a
nice:

    address@hidden tests]$ csi -s scheduler-fdset.scm
    ^Clocking...
    unlocking...
    unlocked...
    done!

Best regards,
-- 
  Jonathan Chan
  address@hidden

On Wed, Jul 8, 2015, at 11:34 PM, Jonathan Chan wrote:
> Hi again,
> 
> It turns out that changing the current thread's state to 'running
> doesn't actually work in all cases because a) unblocking from condition
> variables will skip a thread if its state is running b) the call to
> change the state back was never called because the call to the old hook
> would never return. I found this by doing some more testing with the
> program that caused me to run into the original assert bug.
> 
> Here is a revised patch which instead sets a variable to indicate
> whether or not we are in the interrupt hook and skips the scheduler if
> so. I've also included an updated test case that catches the problem
> with the previous patch.
> 
> Best regards,
> -- 
>   Jonathan Chan
>   address@hidden
> 
> On Wed, Jul 8, 2015, at 08:12 PM, Jonathan Chan wrote:
> > Hi all,
> > 
> > I ended up having some free time before the weekend and went back to the
> > scheduler assert problem. I wrote a test case for the bug and revised
> > the scheduler assert patch after some further testing.
> > 
> > In the previous patch, the thread in which the interrupt handler ran
> > would just be removed from ##sys#fd-list if the block object was changed
> > from a file. This didn't seem to handle some cases where the interrupt
> > handler ended up unblocking. In one of these cases, when mutex-lock! and
> > then mutex-unlock! were called in succession, it also turned out that
> > the "current thread"'s state needed to be saved, set to 'running during
> > the interrupt handler, then restored in order to avoid mutex-unlock!
> > from calling the scheduler and causing the program to hang/do strange
> > things.
> > 
> > I've attached the the test case for the bug and the revised patch.
> > Again, if someone experienced with the scheduler could review it and
> > make sure it doesn't cause strange things to happen, it would be very
> > much appreciated.
> > 
> > Best regards,
> > -- 
> >   Jonathan Chan
> >   address@hidden
> > 
> > On Sat, Jul 4, 2015, at 07:49 PM, Jonathan Chan wrote:
> > > Hello all,
> > > 
> > > I was in the IRC channel earlier trying to find out why my program was 
> > > failing due to a scheduler assertion about the fdset for poll. This 
> > > would happen consistently when I sent SIGINT.
> > > 
> > > The error looks like:
> > > 
> > > scheduler.c:51: C_fd_ready: Assertion `fd == C_fdset_set[pos].fd' failed
> > > 
> > > After looking at the scheduler code, it looks like this happened because 
> > > the SIGINT was causing the interrupt handler to be called, which ended 
> > > up unblocking some thread to run a signal handler, which caused a 
> > > condition variable broadcast. In the end, this made a thread that was 
> > > previously blocking on a file descriptor to now block on a mutex.
> > > 
> > > When create-fdset procedure iterated over the ##sys#fd-list, it skipped 
> > > threads in the list who were not blocking on fds while it added those 
> > > who were to the fdset using fdset-set. However, it did not remove these 
> > > threads from ##sys#fd-list, which resulted in the scheduler throwing an 
> > > error because it expected a fd to be in the fdset when it was not there.
> > > 
> > > I have attached a patch which modifies create-fdset to remove threads 
> > > from ##sys#fd-list who are not blocking on an fd, rather than only skip 
> > > them. This seems to fix my problem and has not introduced any bizarre 
> > > behvaior in the few tests I have run, but hopefully someone who is more 
> > > familiar with the scheduler could look it over and see if it fixes the 
> > > issue.
> > > 
> > > Best regards,
> > > -- 
> > >    Jonathan Chan
> > >    address@hidden
> > > _______________________________________________
> > > Chicken-hackers mailing list
> > > address@hidden
> > > https://lists.nongnu.org/mailman/listinfo/chicken-hackers
> > > Email had 1 attachment:
> > > + scheduler-assert.diff
> > >   2k (text/plain)
> > _______________________________________________
> > Chicken-hackers mailing list
> > address@hidden
> > https://lists.nongnu.org/mailman/listinfo/chicken-hackers
> > Email had 2 attachments:
> > + scheduler-assert-fix.diff
> >   5k (application/octet-stream)
> > + scheduler-fdset.scm
> >   2k (application/octet-stream)
> _______________________________________________
> Chicken-hackers mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/chicken-hackers
> Email had 3 attachments:
> + scheduler-fdset.scm
>   2k (application/octet-stream)
> + sink.c
>   1k (text/plain)
> + scheduler-assert-fix.diff
>   14k (application/octet-stream)

scheduler-assert-fix.diff
Description: Binary data