Skip to content

Race in interrupts?? #2916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
andbjo opened this issue Feb 2, 2017 · 5 comments
Closed

Race in interrupts?? #2916

andbjo opened this issue Feb 2, 2017 · 5 comments

Comments

@andbjo
Copy link

andbjo commented Feb 2, 2017

Basic Infos

Hardware

Hardware: ESP-12E
Core Version: ?2.1.0-rc2?

Description

Problem description
In file: esp8266/cores/esp8266/core_esp8266_wiring_digital.c
In function: __attachInterrupt
the interrupt handler is set before cleaning interrupts for the receive pin witch make the receive sensitive to transitions high to low or low to high.
When this happends the device gets stuck in boot phase.

My proposal is to move
the line interrupt_handler_t *handler = &interrupt_handlers[pin]; to after cleaning interrupt.
As:

extern void ICACHE_RAM_ATTR __attachInterrupt(uint8_t pin, voidFuncPtr userFunc, int mode) {
if(pin < 16) {
// interrupt_handler_t *handler = &interrupt_handlers[pin];
handler->mode = mode;
handler->fn = userFunc;
interrupt_reg |= (1 << pin);
GPC(pin) &= ~(0xF << GPCI);//INT mode disabled
GPIEC = (1 << pin); //Clear Interrupt for this pin
// Put tis line here:
interrupt_handler_t *handler = &interrupt_handlers[pin];
GPC(pin) |= ((mode & 0xF) << GPCI);//INT mode "mode"
}
}

Settings in IDE

Module: NodeMCU
Flash Size: 4MB
CPU Frequency: 80Mhz
Flash Mode: ?qio?
Flash Frequency: 40Mhz
Upload Using: SERIAL/USB
Reset Method: nodemcu

/Anders

@devyte
Copy link
Collaborator

devyte commented Sep 9, 2017

@igrr if this is valid, I think we should give it priority, as it can cause a soft-brick.

@igrr
Copy link
Member

igrr commented Sep 12, 2017

Moving the declaration of handler would not work like that, but thank you for pointing out the issue @andbjo. #3598 has been created to fix this. Please take a look (and try, if you can).

@igrr igrr added this to the 2.4.0 milestone Sep 12, 2017
@andbjo
Copy link
Author

andbjo commented Sep 12, 2017

I know it was not the right way do do it, but it was a quick and dirty fix for me.

@d-a-v
Copy link
Collaborator

d-a-v commented Dec 13, 2017

@andbjo this issue seems to be solved by #3598, can it be closed ?

@andbjo
Copy link
Author

andbjo commented Dec 13, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants