Question about "thread" safe

My ESP32 sketch is receiving commands via bluetooth, and setting a variable interrupt=true to signal a long running process in loop() to exit. Then after exiting loop() sets interrupt=false to reset the state.

However it's theoretically possible for both threads to set that variable at the same time.

Should I be worried about this?

Is there some best practice to handle this kind of thing?

Thanks for any help.

I did not see in your posted code any threads.

loop() and setup() on an ESP32 run on core1, only.

OTA items, when in use, are run on core0.

If you are not using SPIFF's you can gain some flash space by setting the partition scheme to minimal SPIFF's.

Answers no, yes

The ESP32 has a built in multi-threaded, multi-core OS called freeRTOS that can handle multi-processor multi threaded tasks.

I run tasks on both cores, with tasks that have their own core data and, also exchange data across cores.

ESP32 freeRTOS documentation:
https://docs.espressif.com/projects/esp-idf/en/latest/api-reference/system/freertos.html

freeRTOS documentation:

When using freeRTOS loop is not used by the programmer.
Example of using freeRTOS:

#include "esp_system.h" //This inclusion configures the peripherals in the ESP system.
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "freertos/timers.h"
#include "freertos/event_groups.h"
#include <Adafruit_NeoPixel.h>
#include "AudioAnalyzer.h"
////
/* define event group and event bits */
EventGroupHandle_t eg;
#define evtDo_AudioReadFreq       ( 1 << 0 ) // 1
////
TickType_t xTicksToWait0 = 0;
////
QueueHandle_t xQ_LED_Info;
////
const int NeoPixelPin = 26;
const int LED_COUNT = 24; //total number of leds in the strip
const int NOISE = 10; // noise that you want to chop off
const int SEG = 6; // how many parts you want to seperate the led strip into
const int Priority4 = 4;
const int TaskStack40K = 40000;
const int TaskCore1  = 1;
const int TaskCore0 = 0;
const int AudioSampleSize = 6;
const int Brightness = 180;
const int A_D_ConversionBits = 4096; // arduino use 1024, ESP32 use 4096
////
Analyzer Audio = Analyzer( 5, 15, 36 );//Strobe pin ->15  RST pin ->2 Analog Pin ->36
// When we setup the NeoPixel library, we tell it how many pixels, and which pin to use to send signals.
Adafruit_NeoPixel leds = Adafruit_NeoPixel( LED_COUNT, NeoPixelPin, NEO_GRB + NEO_KHZ800 );
////
int FreqVal[7];//create an array to store the value of different freq
////
void setup()
{
  eg = xEventGroupCreate();
  Audio.Init(); // start the audio analyzer
  leds.begin(); // Call this to start up the LED strip.
  clearLEDs();  // This function, defined below, de-energizes all LEDs...
  leds.show();  // ...but the LEDs don't actually update until you call this.
  ////
  xQ_LED_Info = xQueueCreate ( 1, sizeof(FreqVal) );
  //////////////////////////////////////////////////////////////////////////////////////////////
  xTaskCreatePinnedToCore( fDo_AudioReadFreq, "fDo_ AudioReadFreq", TaskStack40K, NULL, Priority4, NULL, TaskCore1 ); //assigned to core
  xTaskCreatePinnedToCore( fDo_LEDs, "fDo_ LEDs", TaskStack40K, NULL, Priority4, NULL, TaskCore0 ); //assigned to core
  xEventGroupSetBits( eg, evtDo_AudioReadFreq );
} // setup()
////
void loop() {} // void loop
////
void fDo_LEDs( void *pvParameters )
{
  int iFreqVal[7];
  int j;
  leds.setBrightness( Brightness ); //  1 = min brightness (off), 255 = max brightness.
  for (;;)
  {
    if (xQueueReceive( xQ_LED_Info, &iFreqVal,  portMAX_DELAY) == pdTRUE)
    {
      j = 0;
      //assign different values for different parts of the led strip
      for (j = 0; j < LED_COUNT; j++)
      {
        if ( (0 <= j) && (j < (LED_COUNT / SEG)) )
        {
          set(j, iFreqVal[0]); // set the color of led
        }
        else if ( ((LED_COUNT / SEG) <= j) && (j < (LED_COUNT / SEG * 2)) )
        {
          set(j, iFreqVal[1]); //orginal code
        }
        else if ( ((LED_COUNT / SEG * 2) <= j) && (j < (LED_COUNT / SEG * 3)) )
        {
          set(j, iFreqVal[2]);
        }
        else if ( ((LED_COUNT / SEG * 3) <= j) && (j < (LED_COUNT / SEG * 4)) )
        {
          set(j, iFreqVal[3]);
        }
        else if ( ((LED_COUNT / SEG * 4) <= j) && (j < (LED_COUNT / SEG * 5)) )
        {
          set(j, iFreqVal[4]);
        }
        else
        {
          set(j, iFreqVal[5]);
        }
      }
      leds.show();
    }
    xEventGroupSetBits( eg, evtDo_AudioReadFreq );
  }
  vTaskDelete( NULL );
} // void fDo_ LEDs( void *pvParameters )
////
void fDo_AudioReadFreq( void *pvParameters )
{
  int64_t EndTime = esp_timer_get_time();
  int64_t StartTime = esp_timer_get_time(); //gets time in uSeconds like Arduino Micros
  for (;;)
  {

    xEventGroupWaitBits (eg, evtDo_AudioReadFreq, pdTRUE, pdTRUE, portMAX_DELAY);
    EndTime = esp_timer_get_time() - StartTime;
    log_i( "TimeSpentOnTasks: %d", EndTime );
    Audio.ReadFreq(FreqVal);
    for (int i = 0; i < 7; i++)
    {
      FreqVal[i] = constrain( FreqVal[i], NOISE, A_D_ConversionBits );
      FreqVal[i] = map( FreqVal[i], NOISE, A_D_ConversionBits, 0, 255 );
      log_i( "Freq %d Value: %d", i, FreqVal[i]);//used for debugging and Freq choosing
    }
    xQueueSend( xQ_LED_Info, ( void * ) &FreqVal, xTicksToWait0 );
    StartTime = esp_timer_get_time();
  }
  vTaskDelete( NULL );
} // fDo_ AudioReadFreq( void *pvParameters )
////
//the following function set the led color based on its position and freq value
void set(byte position, int value)
{
  // segment 0, red
  if ( (0 <= position) && (position < LED_COUNT / SEG) ) // segment 0 (bottom to top), red
  {
    if ( value == 0 )
    {
      leds.setPixelColor( position, 0, 0, 0 );
    }
    else
    {
      leds.setPixelColor( position, leds.Color( value , 0, 0) );
    }
  }
  else if ( (LED_COUNT / SEG <= position) && (position < LED_COUNT / SEG * 2) ) // segment 1 yellow
  {
    if ( value == 0 )
    {
      leds.setPixelColor(position, leds.Color(0, 0, 0));
    }
    else
    {
      leds.setPixelColor(position, leds.Color( value, value, 0)); // works better to make yellow
    }
  }
  else if ( (LED_COUNT / SEG * 2 <= position) && (position < LED_COUNT / SEG * 3) ) // segment 2 pink
  {
    if ( value == 0 )
    {
      leds.setPixelColor(position, leds.Color(0, 0, 0));
    }
    else
    {
      leds.setPixelColor(position, leds.Color( value, 0, value * .91) ); // pink
    }
  }
  else if ( (LED_COUNT / SEG * 3 <= position) && (position < LED_COUNT / SEG * 4) ) // seg 3, green
  {
    if ( value == 0 )
    {
      leds.setPixelColor(position, leds.Color( 0, 0, 0));
    }
    else //
    {
      leds.setPixelColor( position, leds.Color( 0, value, 0) ); //
    }
  }
  else if ( (LED_COUNT / SEG * 4 <= position) && (position < LED_COUNT / SEG * 5) ) // segment 4, leds.color( R, G, B ), blue
  {
    // Serial.println( position );
    if ( value == 0 )
    {
      leds.setPixelColor(position, leds.Color( 0, 0, 0));
    }
    else //
    {
      leds.setPixelColor(position, leds.Color( 0, 0, value) ); // blue
    }
  }
  else // segment 5
  {
    if ( value == 0 )
    {
      leds.setPixelColor(position, leds.Color( 0, 0, 0)); // only helps a little bit in turning the leds off
    }
    else
    {
      leds.setPixelColor( position, leds.Color( value, value * .3, 0) ); // orange
    }
  }
} // void set(byte position, int value)
////
void clearLEDs()
{
  for (int i = 0; i < LED_COUNT; i++)
  {
    leds.setPixelColor(i, 0);
  }
} // void clearLEDs()

That looks like an interesting project!

I'm not referring to anything as exotic as OTA, just receiving a string from a connected bluetooth device, which happens outside of loop(). Something like this:

bool interrupt;

void setup() {


}


void loop() {


	interrupt=false;

	for (i = 0; i < 5000; i++)
		delay(.1);
		if (interrupt) break;
	}   

}



class MyServerCallbacks: public BLEServerCallbacks {
    void onConnect(BLEServer* pServer) {
      btDeviceConnected = true;
    };

    void onDisconnect(BLEServer* pServer) {
      btDeviceConnected = false;
    }
};

class MyCallbacks: public BLECharacteristicCallbacks {
    void onWrite(BLECharacteristic *pCharacteristic) {
      std::string rxValue = pCharacteristic->getValue();

      if (rxValue.length() > 0) {
        Serial.print("BT received: ");

        for (int i = 0; i < rxValue.length(); i++) {
          Serial.print(rxValue[i]);
        }
        Serial.println();

	if (rxValue=="break") interrupt=true;

    }
};

The variable "interrupt" gets set both in loop() and in the bluetooth callback, and they could theoretically be set at the same time. I don't mind if that means I miss a bluetooth value occasionally, but I'm trying to avoid a lockup or a crash.

In the least usevolatile bool interrupt;

If you look at my code, posted above, you can see xQ_LED_Info;. I have the tasks on different cores, and the task trigger each other, so the odds for a shared variable access at the 'same' time are very low but I made the code so that each task can be allowed to run at their own rate; thus, I added in a queue. The queue allows the same data to be used by two differing tasks with each task not interfering with each others operations.

Very interesting about the queue, thanks for that. Also thanks for the tip on "volatile".

I wonder if something like this could solve it.

volatile bool interrupt;
bool write_enabled;

void setup() {


}


void loop() {


	write_enabled=false;
	interrupt=false;
	write_enabled=true;

	for (i = 0; i < 5000; i++)
		delay(.1);
		if (interrupt) break;
	}   

}



class MyServerCallbacks: public BLEServerCallbacks {
    void onConnect(BLEServer* pServer) {
      btDeviceConnected = true;
    };

    void onDisconnect(BLEServer* pServer) {
      btDeviceConnected = false;
    }
};

class MyCallbacks: public BLECharacteristicCallbacks {
    void onWrite(BLECharacteristic *pCharacteristic) {
      std::string rxValue = pCharacteristic->getValue();

      if (rxValue.length() > 0) {
        Serial.print("BT received: ");

        for (int i = 0; i < rxValue.length(); i++) {
          Serial.print(rxValue[i]);
        }
        Serial.println();

	if (rxValue=="break" && write_enabled) interrupt=true;

    }
};

Booleans are atomic. Multiple threads reading and writing to it are thread safe.

delay() takes integers only. Your delay(0.1) is probably turned into delay(0) when compiled (which on the ESP8266 does prevent a WDT timeout, and I assume does the same on the ESP32).