RingBuffer - is inherently unsafe if either store_char or read_char is called from an interrupt.

[KurtE]

Why: The class has a member variable: _numElems
Which is updated by both of these methods.

For example

template <int N>
int RingBufferN<N>::read_char()
{
  if (isEmpty())
    return -1;

  uint8_t value = _aucBuffer[_iTail];
  _iTail = nextIndex(_iTail);
  _numElems--;

  return value;
}

unless _numElems-- is atomic, there is the risk that in between the fetch and store, an interrupt happens which calls store_char one or more times, which each of these calls does numElems++. The store operation will then miss these other changes.

Context: - I am currently experimenting with changes the the SerialX objects for the Uno R4 code base, and was seeing that the processing of interrupts either to store away a character received into one Ring Buffer or to fetch the next byte from the write ring buffer to output, was taking more time than I was expecting. More of this in the issue:
arduino/ArduinoCore-renesas#75

Possible solution: In the past I and I know many others have written buffer classes that the store_char method, would only update the _iHead member variable, and the read_char only updates the _iTail member, and when necessary, you compute how many elements are in the buffer, like:

template <int N>
int SaferRingBufferN<N>::available()
{
  if (_iHead >= _iTail) 
    return _iHead - _iTail;
  return N + _iHead - _iTail;
}

Which is safer. It does not solve the multiple producers or consumers problem.
For those cases you might need something like the SafeRingBuffer code that is in that core. Although I have modified version, which only protects one side. either the Reads or the Writes, where the other side has only the single consumer or producer of the interrupt.
I am testing a version of this now, which appears to be faster.

Should the API version be updated?

[aentinger]

The RingBuffer class is not supposed to be threadsafe which is why i.e. ArduinoCore-renesas has a SafeRingBuffer that provides a interrupt save ringbuffer implementation. So this is very much intented.

[KurtE]

Quick note: I have a quick and dirty test sketch, that is setup for UNOR4 boards:

#include "api/RingBuffer.h"
#include "FspTimer.h"

#define BUF_SIZE 256
RingBufferN<BUF_SIZE> buf;
FspTimer timer;
volatile uint32_t error_count = 0;
volatile uint32_t last_error_count = 0;
uint32_t loop_count = 0;
uint32_t last_report_time = 0;


void setup() {
  while (!Serial && millis() < 5000) 
    ;  // wait for serial...
  Serial.begin(115200);
  pinMode(LED_BUILTIN, OUTPUT);
  digitalWrite(LED_BUILTIN, LOW);

  // get a timer.
  uint8_t type;
  uint8_t ch = FspTimer::get_available_timer(type);
  timer.begin(TIMER_MODE_PERIODIC, type, ch, 10000.0, 50.0, timerISR, nullptr);
  timer.setup_overflow_irq();
  timer.open();
  timer.start();
}

void timerISR(timer_callback_args_t *arg) {
  // lets did quick validate of buffer
  int avail = buf.available();
  int computed_avail = 0;
  if (buf._iHead >= buf._iTail) computed_avail = buf._iHead - buf._iTail;
  else computed_avail = BUF_SIZE + buf._iHead - buf._iTail;
  if (avail != computed_avail) {
    digitalWrite(LED_BUILTIN, HIGH);
    error_count++;
    buf._numElems = computed_avail;
  }

  buf.store_char(loop_count & 0xff);
}

void loop() {
  loop_count++;

  int ch;
  while ((ch = buf.read_char()) != -1);  // may check some here...

  if (error_count != last_error_count) {
    Serial.print("*");
    Serial.print(error_count, DEC);
    Serial.print(" ");
    Serial.println(loop_count, DEC);
    last_error_count = error_count;
    last_report_time = millis();
  }
  if ((millis() - last_report_time) > 10000) {
    Serial.println(loop_count, DEC);
    last_report_time = millis();
    digitalWrite(LED_BUILTIN, LOW);
  }
}

And it does not hit the error condition often, but when it does it would then forever be out of sync.

5900469
11800634
17700746
23600857
29500966
35401079
41301190
*1 41301191
47201226
53101335
59001448
64901559
70801669
76701777
82601889