Flawed Embedded Design Pattern from Claude

An interrupt function reads a byte from a UART and queues it. An interrupt cannot do the data handling as it takes too long and blocks interrupts leading to data loss, so it also posts an action to deal with the data. Only one action should be posted to work the data. The action runs later, outside of interrupts. It will handle all the data in the queue.

Claude enthusiastically generated C code for an interrupt routine and an action with a state machine to govern their interaction:

// ── rx_irq — call from IRQHandler ────────────────
typedef enum { IDLE, QUEUED, RUNNING, REQUEUE, } state_t;
static volatile state_t rx_state = IDLE;
static BQUEUE(100, rxq);
void rx_irq(void) {
    pushbq(ReceiveData8(USART), rxq);
    switch (rx_state) {
        case IDLE:
            rx_state = QUEUED;
            later(rx_action);
            break;
        case RUNNING:
            rx_state = REQUEUE;
            break;
        case QUEUED:
        case REQUEUE:
            break;
    }
}

// ── rx_action single-instance ─────────────────────────
static void rx_action(void) {
    rx_state = RUNNING;
    while (qbq(rxq))  keyIn(pullbq(rxq));

    switch (rx_state) {
        case REQUEUE:
            // Interrupt fired while we were running
            // more bytes may have arrived.
            rx_state = QUEUED;
            later(rx_action);
            break;
        case RUNNING:
     // !!! If interrupt happens right here, 
     //     a character is left in the queue
            rx_state = IDLE;
            break;
        default:
            break;
    }
}        

If the interrupt receives data and the state is IDLE, then an action will be posted to take care of the data. If there already is an action pending, then only data will get queued. When the action runs, it sets the state to RUNNING. If the interrupt occurs, it will set the state to REQUEUE. When the action finishes, but is in the REQUEUE state, it will post itself to the action queue to take care of the data.

At first glance, this all seems good, safe and efficient. Built to order upon request.

But, if one considers what might happen if an interrupt occurred during the state change from RUNNING to IDLE: the state will be put into QUEUED by the interrupt and it won’t post an action because the state reads RUNNING. The action continues unaware  the interrupt has occurred and sets the state to IDLE. There is data queued but no action to take care of it.

In a continuous data flow system, this will just add a delay. A stuck byte that eventually moves through the system. In a request/response packet system, if the last byte gets stuck, it will cause a higher layer protocol layer to time out and deal with it. The probability of the interrupt occurring right at the wrong time will depend on timings of the system. But its occurrence will likely be low and become noise in the system. The system seems to work but there is a tiny flaw.

While Claude can generate code quite easily, detecting flaws still requires a skilled eye. Know your tools so you can trust how you use them.