Firmware: Fixing Interrupt Concurrency Bugs (Spinlock)#

Interrupts Are Special#

For example: 7 (“sensor burning”) beats 42 (timer)
⟶ Best to treat ISRs as “parallel”
⟶ Trust nobody

Low Hanging Fruit#

In FSM::start():

case State::INIT:
    _interval_timer.start(Why::TimeSpec(0,1), _interval);
    Why::IRQ::connect(_sensor_burning_irq, [this](int irqnum){_sensor_burning();});
    Why::IRQ::connect(_operator_button_irq, [this](int irqnum){_operator_button_pressed();});
    _sensor.turn_on();
    _state = State::RUNNING;
    break;

What if timer expires before state is set to RUNNING?
What if we measure before sensor has been turned on?
⟶ Look in FSM::_timer_tick()

Subtle: Interrupt Nesting (7 Beats Timer)#

Sensor blows up right at the same time a “soft overheat” is detected
Timer tick interrupted by higher priority IRQ
State change to SENSORBURNING
Timer tick continues
⟶ Overwrites SENSORBURNING with OVERTEMP
“State Change Overlap”

ISR: Timer tick

ISR: Sensor burning

case State::RUNNING: {
    double temp = _sensor.get_temperature();

    if (temp > _soft_critical_temp) {
        _cooling.set_state(true);
.
.
.
.
.
.
        _soft_alarm.set_state(true);
        _state = State::OVERTEMP;
    }
    ...
    break;

.
.
.
.
.
case State::RUNNING:
    _sensor.turn_off();
    _cooling.set_state(false);
    _state = State::SENSORBURNING;
    _hard_alarm.set_state(true);
    break;
.
.
.
.
.

Effects Of Unprotected Interrupt Nesting#

../../../../../../../_images/fsm-parallel.svg

Protecting Against Unwanted Interrupt Nesting#

Turn off interrupts before critical section is entered
But: there is a better too that does more
… and also works on a multicore CPU
⟶ Spinlock

Critical Section?#

Look in state machine
First: FSM::_timer_tick()
What is critical? Remember problem: in RUNNING, we got interrupted.
Here’s the relevant code from Finite State Machine (fsm.h, fsm.cpp)

switch (_state) {
    ...
    case State::RUNNING: {
        double temp = _sensor.get_temperature();

        if (temp > _soft_critical_temp) {
            _cooling.set_state(true);
            _soft_alarm.set_state(true);
            _state = State::OVERTEMP;
        }
        else if (temp > _high_temp)
            _cooling.set_state(true);
        else if (temp < _low_temp)
            _cooling.set_state(false);
        break;
    }
    ...
}

Updated FSM: Spinlock#

code/fsm.h#

#pragma once

#include "sensor.h"
#include "switch.h"

#include <why-timer.h>
#include <why-spinlock.h>                              // <-- pull in spinlock


class FSM
{
public:
    enum class State {
        INIT,
        RUNNING,
        OVERTEMP,
        SENSORBURNING
    };
    
public:
    FSM(Why::TimeSpec interval,
        int sensor_burning_irq, Switch& hard_alarm,
        long soft_critical_temp, Switch& soft_alarm,
        int operator_button_irq,
        Sensor& sensor, Switch& cooling,
        long low_temp, long high_temp);

    void start();

private:
    void _timer_tick();
    void _sensor_burning();
    void _operator_button_pressed();

private:
    Why::Spinlock _lock;                               // <-- essential class member!

    Why::TimeSpec _interval;
    Why::Timer _interval_timer;
    int _sensor_burning_irq;
    Switch& _hard_alarm;
    long _soft_critical_temp;
    Switch& _soft_alarm;
    int _operator_button_irq;
    Sensor& _sensor;
    Switch& _cooling;
    long _low_temp;
    long _high_temp;

    State _state;
};

code/fsm.cpp#

#include "fsm.h"

#include <why-irq.h>

#include <cassert>


FSM::FSM(
    Why::TimeSpec interval,
    int sensor_burning_irq, Switch& hard_alarm,
    long soft_critical_temp, Switch& soft_alarm,
    int operator_button_irq,
    Sensor& sensor, Switch& cooling,
    long low_temp, long high_temp)
: _interval(interval),
  _interval_timer([this](){_timer_tick();}),
  _sensor_burning_irq(sensor_burning_irq),
  _hard_alarm(hard_alarm),
  _soft_critical_temp(soft_critical_temp),
  _soft_alarm(soft_alarm),
  _operator_button_irq(operator_button_irq),
  _sensor(sensor), 
  _cooling(cooling), 
  _low_temp(low_temp), 
  _high_temp(high_temp), 
  _state(State::INIT)
{}

void FSM::start()
{
    _lock.lock();                                      // <-- acquire
    switch (_state) {
        case State::INIT:
            _interval_timer.start(Why::TimeSpec(0,1), _interval);
            Why::IRQ::connect(_sensor_burning_irq, [this](int irqnum){_sensor_burning();});
            Why::IRQ::connect(_operator_button_irq, [this](int irqnum){_operator_button_pressed();});
            _sensor.turn_on();
            _state = State::RUNNING;
            break;
        case State::RUNNING:
            assert(!"RUNNING");
            break;
        case State::OVERTEMP:
            assert(!"OVERTEMP");
            break;
        case State::SENSORBURNING:
            assert(!"SENSORBURNING");
            break;
    }
    _lock.unlock();                                    // <-- release
}

void FSM::_timer_tick()
{
    _lock.lock();                                      // <-- acquire
    switch (_state) {
        case State::INIT:
            assert(!"INIT");
            break;
        case State::RUNNING: {
            double temp = _sensor.get_temperature();
            
            if (temp > _soft_critical_temp) {
                _cooling.set_state(true);
                _soft_alarm.set_state(true);
                _state = State::OVERTEMP;
            }
            else if (temp > _high_temp)
                _cooling.set_state(true);
            else if (temp < _low_temp)
                _cooling.set_state(false);
            break;
        }
        case State::OVERTEMP: {
            double temp = _sensor.get_temperature();
            
            if (temp > _high_temp)
                _cooling.set_state(true);
            else if (temp < _low_temp)
                _cooling.set_state(false);
            break;
        }
        case State::SENSORBURNING:
            break;
    }
    _lock.unlock();                                    // <-- release
}
    
void FSM::_sensor_burning()
{
    _lock.lock();                                      // <-- acquire
    switch (_state) {
        case State::INIT:
            assert(!"INIT");
            break;
        case State::RUNNING:
            _sensor.turn_off();
            _cooling.set_state(false);
            _hard_alarm.set_state(true);
            _state = State::SENSORBURNING;
            break;
        case State::OVERTEMP:
            _sensor.turn_off();
            _cooling.set_state(false);
            _soft_alarm.set_state(false);
            _hard_alarm.set_state(true);
            _state = State::SENSORBURNING;
            break;
        case State::SENSORBURNING:
            break;
    }
    _lock.unlock();                                    // <-- release
}

void FSM::_operator_button_pressed()
{
    _lock.lock();                                      // <-- acquire
    switch (_state) {
        case State::INIT:
            assert(!"INIT");
            break;
        case State::RUNNING:
            break;
        case State::OVERTEMP:
            _soft_alarm.set_state(false);
            _state = State::RUNNING;
            break;
        case State::SENSORBURNING:
            break;
    }
    _lock.unlock();                                    // <-- release
}