sfos/ecl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix spinlock handling in queue_signal and pop_signal

Browse source 
We can't use the ECL_WITH_SPINLOCK_BEGIN/END macros since they check
for pending interrupts at the end of their unwind-protect frame. This
leads to various bugs:

- in queue_signal the to be queued interrupt is executed immediately
  after being queued even if interrupts are disabled
- in pop_signal if multiple interrupts are queued they are executed in
  reverse order

To fix these issues, use a) ecl_get/giveup_spinlock directly and b)
ecl_disable/enable_interrupts_env to prevent the spinlock not being
released due to an interrupt happening during the execution of the
main body of pop_signal/queue_signal.
This commit is contained in:
Marius Gerbershagen 2020-06-01 17:18:35 +02:00
parent e89dce9631
commit ed11e2cc35
1 changed files with 56 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

84
src/c/unixint.d
View file

@ -425,43 +425,66 @@ handle_all_queued_interrupt_safe(cl_env_ptr env)
static void
queue_signal(cl_env_ptr env, cl_object code, int allocate)
{
ECL_WITH_SPINLOCK_BEGIN(ecl_process_env(), &env->interrupt_struct->signal_queue_spinlock) {
cl_object record;
if (allocate) {
record = ecl_list1(ECL_NIL);
} else {
record = env->interrupt_struct->signal_queue;
if (record != ECL_NIL) {
env->interrupt_struct->signal_queue = ECL_CONS_CDR(record);
}
}
/* Note: We don't use ECL_WITH_SPINLOCK_BEGIN/END here since
* it checks for pending interrupts after unlocking the
* spinlock. This would lead to the interrupt being handled
* immediately when queueing an interrupt for the current
* thread, even when interrupts are disabled. */
/* INV: interrupts are disabled, therefore the spinlock will
* always be released */
ecl_get_spinlock(ecl_process_env(), &env->interrupt_struct->signal_queue_spinlock);
cl_object record;
if (allocate) {
record = ecl_list1(ECL_NIL);
} else {
record = env->interrupt_struct->signal_queue;
if (record != ECL_NIL) {
ECL_RPLACA(record, code);
env->interrupt_struct->pending_interrupt =
ecl_nconc(env->interrupt_struct->pending_interrupt,
record);
env->interrupt_struct->signal_queue = ECL_CONS_CDR(record);
}
} ECL_WITH_SPINLOCK_END;
}
if (record != ECL_NIL) {
ECL_RPLACA(record, code);
env->interrupt_struct->pending_interrupt =
ecl_nconc(env->interrupt_struct->pending_interrupt,
record);
}
ecl_giveup_spinlock(&env->interrupt_struct->signal_queue_spinlock);
}
static cl_object
pop_signal(cl_env_ptr env)
{
cl_object record, value;
ECL_WITH_SPINLOCK_BEGIN(env, &env->interrupt_struct->signal_queue_spinlock) {
if (env->interrupt_struct->pending_interrupt == ECL_NIL) {
value = ECL_NIL;
} else {
record = env->interrupt_struct->pending_interrupt;
value = ECL_CONS_CAR(record);
env->interrupt_struct->pending_interrupt = ECL_CONS_CDR(record);
/* Save some conses for future use, to avoid allocating */
if (ECL_SYMBOLP(value) || ECL_FIXNUMP(value)) {
ECL_RPLACD(record, env->interrupt_struct->signal_queue);
env->interrupt_struct->signal_queue = record;
}
/* Note: We don't use ECL_WITH_SPINLOCK_BEGIN/END here since
* it checks for pending interrupts after unlocking the
* spinlock. This would lead to handle_all_queued and
* pop_signal being called again and the interrupts being
* handled in the wrong order. */
/* INV: ecl_get_spinlock and ecl_giveup_spinlock don't write
* into env, therefore it is valid to use
* ecl_disable_interrupts_env */
ecl_disable_interrupts_env(env);
ecl_get_spinlock(env, &env->interrupt_struct->signal_queue_spinlock);
if (env->interrupt_struct->pending_interrupt == ECL_NIL) {
value = ECL_NIL;
} else {
record = env->interrupt_struct->pending_interrupt;
value = ECL_CONS_CAR(record);
env->interrupt_struct->pending_interrupt = ECL_CONS_CDR(record);
/* Save some conses for future use, to avoid allocating */
if (ECL_SYMBOLP(value) || ECL_FIXNUMP(value)) {
ECL_RPLACD(record, env->interrupt_struct->signal_queue);
env->interrupt_struct->signal_queue = record;
}
} ECL_WITH_SPINLOCK_END;
}
ecl_giveup_spinlock(&env->interrupt_struct->signal_queue_spinlock);
ecl_enable_interrupts_env(env);
return value;
}
@ -1076,8 +1099,13 @@ ecl_interrupt_process(cl_object process, cl_object function)
if (!Null(function) &&
(process->process.phase >= ECL_PROCESS_BOOTING))
{
cl_env_ptr the_env = ecl_process_env();
function = si_coerce_to_function(function);
/* queue_signal must be called with disabled
* interrupts for the current process */
ecl_disable_interrupts_env(the_env);
queue_signal(process->process.env, function, 1);
ecl_enable_interrupts_env(the_env);
}
/* ... but only deliver if the process is still alive */
if (process->process.phase == ECL_PROCESS_ACTIVE)