`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?

[MaBecker]

look's like this code is helpfull to fix this issue.