[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Chicken-hackers] [PATCH] [PRERELEASE] Fix #1283
From: |
Peter Bex |
Subject: |
[Chicken-hackers] [PATCH] [PRERELEASE] Fix #1283 |
Date: |
Sun, 24 Apr 2016 15:07:23 +0200 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
Good news everyone!
I managed to pinpoint the cause of #1283, and it's so simple I'm kind
of surprised this hasn't been found earlier.
When a process receives a POSIX signal, C_raise_interrupt will
modify C_stack_limit to be identical to stack_bottom, which means
the very next stack check in C_demand() will fail, causing a GC
to be triggered which then will reset the stack and pick the pending
interrupts from the list.
This is really a bit of an abuse/overloading of the C_stack_limit
variable, and it is biting us in the behind pretty badly now:
The C_stack_check1() macro also uses C_stack_limit. As you know, this
macro is used in equal? to "detect" cyclic or very deeply nested
structures, and in C_stack_overflow_check(), which is inserted by the
compiler to allow programs to recover from detected runaway recursions
by raising a condition.
This means that if a signal arrives in between a C_demand() and a
C_stack_overflow_check() call, the latter will trigger the aforementioned
condition signaling.
As far as I understand it, this problem has always lurked in the code
base. I guess we don't run into this very often because a typical
process won't receive that many signals, and it's a big coincidence if
it will happen before C_stack_check1() instead of C_demand() (the latter
occurs much more often). This is also the reason it got triggered in
the scsh-process tests: it will receive SIGCHLD when a child is finished,
and it does spawn quite a few child processes.
The attached patch is a simple one, it simply replaces all C_stack_limit
by C_stack_hard_limit, which indicates the actual limit. C_stack_limit
is initialised to C_stack_hard_limit, so it will have the same meaning as
before. The signal handler and C_demand code still use C_stack_limit, so
we can keep relying on this trick to force a GC. One very nice advantage
of this is that we can get rid of the saved_stack_limit: we can simply
set C_stack_limit to C_stack_hard_limit when we need to restore it.
I'm not super-happy with the fact that we are mutating C_stack_limit like
we do to enforce a GC, but this has been done for performance reasons I
think: only one variable needs to be checked in C_demand. Of course
C_demand is used EVERYWHERE, so it really needs to be fast. For this
reason I decided to keep the existing mechanism, and also to keep the
patch as simple and understandable as possible, so it doesn't involve
a complete code overhaul just before the second release candidate.
The attached patches should go into prerelease, master and chicken-5.
Cheers,
Peter
0001-Avoid-triggering-stack-overflows-in-signal-handler.chicken-5.patch
Description: Text Data
0001-Avoid-triggering-stack-overflows-in-signal-handler.master-prerelease.patch
Description: Text Data
signature.asc
Description: Digital signature
- [Chicken-hackers] [PATCH] [PRERELEASE] Fix #1283,
Peter Bex <=