FreeRTOS deadlock on mutex_enter_blocking/recursive_mutex_enter_blocking

[godario]

Hi!
Using FreeRTOS and making intensive use of multiple String objects and Serial communication I noticed a deadlock during calls to mutex_enter_blocking()/recursive_mutex_enter_blocking()
Using platformio and debugging with picoprobe I get the calls stack leading to deadlock: attached two cases as screenshots inside the archive.
I am attaching a MCVE tested both with Arduino IDE and with VSCode & platform.io using the core version 2.4.1.
Can anything be done to prevent this problem from happening? Thank you!

mutex_enter_stuck_MCVE.zip

[earlephilhower]

Thanks for the short-and-sweet MCVE. I especially liked the variable names.

The 0-effort-on-my-part solution is for you to use a FreeRTOS mutex to protect any writes to the Serial device. That's the general rule you should be following, anyway, as >99% of the libraries on Arduino do not understand multithreaded, let alone multicore.

That said, I implemented a simple mutex for multicore, but not multithreaded, support on the Serial port and that handles the non-FreeRTOS case since there's only 2 jobs in the whole system. Did this special case because it was so darn useful to let both cores print stuff.

I haven't run the MCVE, but thinking about it my guess is one task got the serial mutex but got swapped out and another (one or two) are trying for it and locking interrupts waiting for the mutex to free. When IRQs are locked then the "swap task" interrupt can't reach core 1 and you're stuck.

The change-the-core method probably involves a special codepath on that CoreMutex object to make it use the FreeRTOS mutex (which understands priority inversion/etc.) when in FreeRTOS mode. It's something that's been at the back of my mind for a while, but not something I've had time to look in to yet.

[godario]

Thanks for the detailed answer.
I suspected that Serial was core-safe between the 2 hardware cores but not between FreeRTOS threads however, I did not imagine that the same problem also afflicted the functions for dynamic allocation.

In fact, as you can see from this other MCVE sketch, the same problem occurs if I call the malloc from multiple threads (running from both cores). To confirm this, by wrapping malloc and free with a mutex (enabling useMutex) the execution proceeds without any block.

While I find it more than fair to use a mutex for serial access, I find it somewhat impractical to wrap calls to any library that is supposed to use malloc/calloc/etc. In this case, what do you think would be a good solution at both the sketch and core level?

In the past, on FreeRTOS for SAMD21 (b6f4240), I had solved by wrapping the heap functions from the linker in order to use those of FreeRTOS (in that case I had also implemented calloc and realloc since FreeRTOS does not have a thread-safe version of them): do you think this could be a valid and viable path also in this case?

Thank you again!

mutex_enter_stuck_malloc_MCVE.zip

[earlephilhower]

Thanks again for a concise MCVE! I agree about malloc, it should be safe (even if runtime is unbounded and shouldn't be used in any real hard realtime stuff).

These all fall into the same bucket, namely patching in FreeRTOS mutexes for system protection over the standard SDK ones. Malloc (and a bunch of other parts of the standard library) is mutex protected by Newlib in cores/rp2040/lock.cpp which needs to be made FreeRTOS aware.

It's definitely top of the queue for me given the severity and reproducibility w/MCVEs, but it's probably going to take until the weekend before I can start experimenting.

[earlephilhower]

Can you please give #798 a try, @godario ? It seems to clear up hangs with the malloc MCVE (not the SerialUART one originally posted). It is a bit of a heavy hammer but it is the most straightforward to implement. I believe there is no mutex nesting in newlib, so it should be safe.

[earlephilhower]

@godario it seems #798 also clears up your Serial1 issue. I've tested w/o and it hangs and mixes up serial streams, and with the PR it runs forever and the lines are properly separated.

So, I think we're good on both, at least w/the MCVEs given.

[godario]

Well, I tried to do more in-depth tests, I confirm that with the two previous MCVEs everything seems to work, but with the sketch with which for the first time I got this problem I still have the same problem (it's a stress test for a library which makes intensive use of Serial and String). For this reason I have written a new MCVE which, in addition to malloc and free, also calls the realloc (widely used by the String object): unfortunately, this MCVE shows that the problem persists.

A few details about this MCVE:
The call to realloc can be enabled or disabled through the tryRealloc flag.

The acquisition of the mutex near each of the heap functions can be enabled or disabled through the useMutexOnMalloc, useMutexOnRealloc and useMutexOnFree flags.

I have implemented the possibility of using a separate mutex for each of the functions that use the heap, in this malloc, realloc and free. Since the bug in question occurs when any two of these functions are executed at the same time, at the moment I have proceeded to use a single mutex for all of them (in a not exactly elegant way).

To replicate the problem with this new MCVE it is necessary to activate tryMutex and disable the use of the mutex on all functions of the heap (the sketch I am attaching already respects this configuration). Note that activating the mutex only for the realloc does not solve the problem because clearly this would not guarantee that a call to the realloc is not executed at the same time as a free or a malloc.

Thanks!

mutex_enter_stuck_realloc_MCVE.ino.zip

[earlephilhower]

We're getting there, if slowly. I can repro w/your MCVE, thx.

A quick trace shows that it's stuck in your setup() call with the malloc recursive mutex.

My thinking is that FreeRTOS's do-not-preempt calls are not really counters and I need to track my own. The only way I can see malloc getting into that state is to have a recursive release (where count is still > 0) which sends the enable-preempt.

I'll see what I put in. Might simply be checking the count of the recursive mutex before re-enabling preemption.

[earlephilhower]

Err, and that recursive part is the issue, I think. REcursive mutexes in the SDK identify the owner by the Core ID....but w/multithread you need to use the thread ID or two threads on the same core can succeed in the recursive call. Looks like I need real FreeRTOS recursive semaphores, ugh.