Interrupt race condition on RX buffer
AzwarPrihastomo opened this issue · 3 comments
I found issue when using multiple can port to listen bus at high rate
i found that the way interrupt store data using this
FCTP_FUNC void FCTP_OPT::struct2queueRx(const CAN_message_t &msg) {
uint8_t buf[sizeof(CAN_message_t)];
memmove(buf, &msg, sizeof(msg));
rxBuffer.push_back(buf, sizeof(CAN_message_t));
}
the problem is when we calling FCTP_FUNC uint64_t FCTP_OPT::events() this function is accessing rxBuffer may make race condition
so i'm propose to add guard when we access rxBuffer by disabling the interrupt
like below
FCTP_FUNC uint64_t FCTP_OPT::events() {
if ( rxBuffer.size() ) {
CAN_message_t frame;
uint8_t buf[sizeof(CAN_message_t)];
NVIC_DISABLE_IRQ(nvicIrq);
rxBuffer.pop_front(buf, sizeof(CAN_message_t));
NVIC_ENABLE_IRQ(nvicIrq);
memmove(&frame, buf, sizeof(frame));
mbCallbacks((FLEXCAN_MAILBOX)frame.mb, frame);
}
if ( txBuffer.size() ) {
CAN_message_t frame;
uint8_t buf[sizeof(CAN_message_t)];
txBuffer.peek_front(buf, sizeof(CAN_message_t));
memmove(&frame, buf, sizeof(frame));
if ( write((FLEXCAN_MAILBOX)getFirstTxBox(), frame) ) txBuffer.pop_front();
}
return (uint64_t)(rxBuffer.size() << 12) | txBuffer.size();
}
it's a FIFO stack, interrupts queue to end, events pulls from front, no locks are necessary
the buffer that you use is not thread safe, so the head and tail index could be prone to race condition because of this interrupt
it's not thread safe, it's context safe, no need to block interrupts. each bus has their own rx buffer. the interrupt writes to the back of the queue, the callback pulls from the front, thats how the FIFO queue works