johnnyb / Eventually

A library for event-based programming to make Arduino programming more fun and intuitive
MIT License
190 stars 41 forks source link

Example with mgr.resetContext() misleading, leads to exception and program termination #13

Open cwienands1 opened 3 years ago

cwienands1 commented 3 years ago

I started out with the example code from the Readme page and from there started developing some more complex scenario involving two timers. I quickly stumbled across the issues already reported related to the addListeners method, found another problem with non-multiFire listeners (added comment in existing issue report) but then came across a much more serious issue that crashes the executing program with an exception.

Per readme, I figured I can kill all existing timers with mgr.resetContext() from within a callback method.

bool start_blinking() {
  mgr.resetContext(); 
  mgr.addListener(new EvtTimeListener(500, true, (EvtAction)blink_pin));
  mgr.addListener(new EvtPinListener(BUTTON_PIN, (EvtAction)stop_blinking));
  return true;
}

The problem is that as part of the mgr.resetContext() method, the existing listener objects get deleted, while start_blinking() is called FROM a listener object. Once the callback method, here start_blinking, has completed the handleTimerEvent() still accesses member variables of the listener object like multiFire and even calls the non-static method setupListener() on a deleted object. Per CPP spec, this is undefined behavior. During my experiments I see an exception with subsequent termination of the program.

My background is managed languages (C#, JS, Java) so I'm still trying to wrap my head around this, trying to figure out how this chicken-egg problem can be avoided. I guess the listener deletion would have to be deferred until the performTriggerAction method has completed.

In the meantime, do NOT use the construct that is shown in the Readme.