taskENABLE_INTERRUPTS/taskDISABLE_INTERRUPTS causing panic resets on ESP32

[idobh2]

I'm seeing an issue with interrupts on ESP32, causing panic resets.
My js code is pretty simple:

setWatch(() => {console.log("change")}, D21, { repeat: true, });

After a few log lines, I'm seeing the following error dumped
Guru Meditation Error: Core 0 panic'ed (StoreProhibited). Exception was unhandled.
With some cpu register dump and then a reboot.

After some digging, I'm pretty sure the cause to this error are the calls to taskENABLE_INTERRUPTS() and taskDISABLE_INTERRUPTS(); in jshInterruptOn and jshInterruptOff respectively, under the targets/esp32 implementaion.

When I did the following (probably very irresponsible) change locally to the targets/esp32/jshardware.c, I stopped getting these panics, and interrupts worked well

1. First I defined a new global boolean

static bool intr_enabled = true;

2. Then made sure the gpio_intr_handler doesn't push a new event if intr_enabled is false. this is done only after the interrupt is read and cleared

static void IRAM_ATTR gpio_intr_handler(void* arg){
  //GPIO intr process. Mainly copied from esp-idf
  UNUSED(arg);
  IOEventFlags exti;
  Pin gpio_num = 0;
  uint32_t gpio_intr_status = READ_PERI_REG(GPIO_STATUS_REG);   //read status to get interrupt status for GPIO0-31
  uint32_t gpio_intr_status_h = READ_PERI_REG(GPIO_STATUS1_REG);//read status1 to get interrupt status for GPIO32-39
  SET_PERI_REG_MASK(GPIO_STATUS_W1TC_REG, gpio_intr_status);    //Clear intr for gpio0-gpio31
  SET_PERI_REG_MASK(GPIO_STATUS1_W1TC_REG, gpio_intr_status_h); //Clear intr for gpio32-39
  if(!intr_enabled){
    return;
  }
  do {
    g_pinState[gpio_num] = 0;
    // ...

3. Then I modified jshInterruptOff and jshInterruptOn to change intr_enabled instead of calling taskDISABLE_INTERRUPTS/taskENABLE_INTERRUPTS

void jshInterruptOff() { 
  //taskDISABLE_INTERRUPTS();
  intr_enabled = false;
}

void jshInterruptOn()  {
  //taskENABLE_INTERRUPTS();
  intr_enabled = true;
}

This resulted in consistent operation of interrupts and got the setWatch working properly.
That said, I guess my code is really naïve, as i'm not too familiar with the internals of the ESP32 libs and parallelism in this specific case.
I guess there's a more elegant solution here, but if this is something you think should be introduced as presented above, I'd be happy to create a PR,.

[hagai-shatz]

Seems like using taskENABLE_INTERRUPTS/taskDISABLE_INTERRUPTS is not a valid method on multi-core, see FreeRTOS documentation:

Warning
Disabling interrupts is a valid method of achieve mutual exclusion in Vanilla FreeRTOS (and single core systems in general). However, in an SMP system, disabling interrupts is NOT a valid method ensuring mutual exclusion. Refer to Critical Sections for more details.

Seems like a change is needed to use taskENTER_CRITICAL(&spinlock) / taskEXIT_CRITICAL(&spinlock) (or taskENTER_CRITICAL_ISR(&spinlock) / taskEXIT_CRITICAL_ISR(&spinlock) from an interrupt context).

[MaBecker]

@idobh2 does your changes work for multiple setwatches too?