I have the the following situation: An STM32L476RG (ARM Cortex M4F @ 80MHz, 1MB Flash, 128kB RAM) parses some string data coming in over UART. The project uses mbed-os. The parsing of the data is done in an interrupt-based manner, meaning an ISR is in place which reads the just received character and has some stateful parsing logic to extract data from it. While doing so, it modifies an working-object which gradually gets filled up with the parsed values.
When the parsing process is done (all values filled), the working copy is saved into another global object of the same type. This is so that the application has an object with valid values and doesn't have to access the working copy of the ISR-based parser, which might change anytime. The structure in question is about 204 bytes big.
Code looks like (high level)
struct MeterState {
double valueA;
double valueB;
double valueC;
char id[64];
//more..
}
volatile MeterState isrWorkingCopy;
volatile MeterState lastValue;
/* called in an ISR context by mbed-os whenever something is received on the serial */
static void SerialParserHandler() {
uint8_t data = (uint8_t) meterSerial.getc();
parse_data(data, &isrWorkingCopy);
//check parsing finished..
if(parsingFinished) {
//save finished object
lastValue = isrWorkingCopy;
}
}
/* called by application to get last valid data */
void MeterState GetLastValue() {
return lastValue; //copy structure to as return value to stack
}
However, here comes the crux of the problem. Due to length of the object and given some certain timing, it may be possible that:
- Appcode calls
GetLastValue()
- Function starts copying the huge structure to the stack of the caller.
- Mid-term gets interrupted by the ISR
- ISR overwrites
lastValue
with new data when parsing is finished GetLastValue()
is resumed after ISR- Resulting returned value is half the old half the new value
See e.g. compiler output for GetLastValue()
, it is implemented by a memcpy()
:
--
08015f3c <_ZN13ElectricMeter16GetLastValueEv>:
8015f3c: b508 push {r3, lr}
8015f3e: 22d0 movs r2, #208 ; 0xd0
8015f40: 4901 ldr r1, [pc, #4] ; (8015f48 <_ZN13ElectricMeter16GetLastValueEv+0xc>)
8015f42: f004 fa5f bl 801a404 <memcpy>
8015f46: bd08 pop {r3, pc}
8015f48: 20001d38 .word 0x20001d38
Basically overwriting and getting the value is non-atomic. I'm not quiet seeing how it's possible to make it atomic so that the above scenario doesn't occur anymore.
I cannot just use a Mutex-based locking mechanism to atomatically exchange the lastValue
because I cannot lock a mutex inside an ISR. The ISR must be non-blocking.
I also thought about temporarily disabling interrupts around the getter method as such
MeterState GetLastValueSafe() {
__disable_irq();
MeterState s = GetLastValue();
__enable_irq();
return s;
}
However, doesn't this have the possibility (as small as it may be) that the parser misses a character (which might screw up parsing and constitute a lost data value)? If the IRQs are disabled and the handler doesn't get invoked, the character is basically lost, as I understand.
Is there any sane way to solve this general problem of ISR concurrency with large structures?
Best Answer
Assuming that your parsing routine is fast (you really should minimize time spend inside an interrupt), I'd consider modifying your code, as follows.
The above technique only declares one pointer as volatile; the one that can be modified by the interrupt routine. There's no need to declare anything else in that way.
The above code also makes the interrupt routine solely responsible for updating the unfilled variable. No one else has any right to modify it. They are only allowed to observe it. That's all. (It's volatile, of course, since the interrupt might occur at any time and update it.)
It also makes the GetLastValue function solely responsible for updating the filled variable. No one else (not even the interrupt code) has any right to modify it. Others may only observe it. There's no need for it to be volatile as any modification by GetLastValue is done outside of any interrupt events (unless you do something to hook it into one, I suppose.)
By separating ownership of these pointers, there's never a problem with processing and updating their values.
I added an Increment() macro for clarity. You can expand it inline if you want.
There is a potential problem in the above code, highlighted by my comment. If the interrupt routine executes sends out two filled buffers before GetNextValue() has a chance to fetch one, then the double-buffer is insufficient and there really isn't a good way to recover from that. Which is why I suggested a circular buffer (of larger size.) You can modify the above code easily by increasing the number of MeterBuffer's and reduce (but never eliminate) the risks here. Only you can work out what you want to do if this problem happens. So I leave that to you to worry about.