bblanchon / ArduinoStreamUtils

💪 Power-ups for Arduino streams
MIT License
257 stars 21 forks source link

multicore environment: locking of Stream methods #33

Closed mhaberler closed 1 year ago

mhaberler commented 1 year ago

Hi Benoit,

not really your issue or an issue of this wonderful library, but rather a general Arduino problem with multicore environments like ESP32 - really just putting it up for discussion

I am prepared to roll my own solution, so no fear of feature requests ;) but this is going to bite others as well

I have the following setup:

reasoning: I want console output go to Serial, and to any WebSerial clients as-is. The single character I/O of console output is killing the Websockets (really ESPAsyncWebserver) and its not her fault, so I introduced buffering. To get stuff out eventually, I added a flushTicker to flush the whole stack of streams periodically. This code is not in the online version but looks like so:

  // during setup run flushBuffers(); via a ticker
  // at the end of setup detach the ticker as loop() will take over
  flushTicker.attach_ms(STARTUP_FLUSH_INTERVAL, []() { flushBuffers(); });

That buffering works fine... mostly. Every now and then output becomes intermingled and garbled.

I tracked down the issue to the ticker library running its own high-priority soft timer task pinned to esp32 core 0 which actually calls the user callbacks when due - in the context of core 0. Application code like setup() and loop() are pinned to core 1.

The garbling seems to happen when write() calls are percolating down this stack of streams, and ticker kicks in and calls flushBuffers() which calls the stream's flush() method simultaneously. I am pretty sure that is the cause, because if I flushBuffers() repeatedly at some points in setup() the problem goes away.

What this boils down to is: Arduino Stream, Print etc are not multicore-safe as-is, and likely not even single-core multithreading with a preemptive OS underneath.

The solution of course is: use a real operating system ;)

The second-best solution is to introduce locking on Stream methods. Now doing this across the board would be a tad heavy-handed, so I am looking at a wrapper class which can be used as poor man's multicore safety feature selectively.

That suggests a wrapper class which has a mutex, and calls to write(), flush() etc need to acquire the mutex on enter, and release on return. Waiting delay, or no-wait immediate failure would be parameters; for instance, if the ticker-driven flush() routine happens to hit a locked mutex no harm is done by immediately returning - flushing eventually is good enough.

For FreeRTOS, I have the nicely wrapped mutex primitives at hand.

Have you ever seen something similar? What would you suggest?

best regards Michael

bblanchon commented 1 year ago

Hi @mhaberler,

I don't know if it's luck or talent, but I always managed to avoid this kind of issue, at least with Arduino. My strategy is to do as much as possible in the main loop and as little as possible in the interrupt handlers. Would this strategy be able to solve your issue? Maybe not, but it's worth a try. Start by moving the call to flushBuffers(); to the main loop...

Now, I'm not sure what you expect from me. I suppose you'd like to see a new Stream decorator that uses a mutex to synchronize all the calls, but that poses two questions. What mutex implementation would it use? How do we share the mutex between the instances?

I don't believe this would work, and even if it did, I don't want to include this in the library because I don't want my users to think it can magically fix concurrency issues. Stream is not thread-safe, so you should add the synchronization mechanism in your code if you need to use it from multiple threads.

Best regards, Benoit

mhaberler commented 1 year ago

I did not suggest any action, but your senior advice is appreciated - I basically was probing if a Stream wrapper makes sense

I decided against fixing the issue, and went with flushing inline

the whole issue is during setup() only, in loop() periodic flushing is in place output just stuttered during setup() and I wanted to see what's going on.

thanks!