Use an external function to take a mutex within a task

Question

I was wondering if I can take a mutex within a task but by calling an external function. Here is my code below:

void TakeMutexDelay50(SemaphoreHandle_t mutex)
{
    while(xSemaphoreTake(mutex, 10) == pdFALSE)
    {
        vTaskDelay(50);
    }
}

bool ContinueTaskCopy()
{
    TakeMutexDelay50(ContinueTask_mutex);
    bool Copy = ContinueTask;
    xSemaphoreGive(ContinueTask_mutex);
    return Copy;
}

Basically, my task calls the function ContinueTaskCopy(). Woud this be good practice?

Not yet. I'm still writing the code. I wanted to make sure it's good practice before putting it everywhere in my tasks — hlarouss, Oct 07 '19 at 11:20
It's not clear what your doubt about this is. A task is not restricted to a single function. Functions execute within the context of the calling task. In other words, it's still the same task executing within the sub-function(s). — kkrambo, Oct 07 '19 at 11:58
The context was exactly my source of doubt! I wasn't sure whether the scheduler would see it as the same context or not :) — hlarouss, Oct 07 '19 at 15:42

score 1 · Answer 1 · answered Oct 07 '19 at 09:26

1

The code above will work, but if you are not doing anything in the while loop for taking the mutex you could just set the timeout to portMAX_DELAY and avoid all the context switches every 50 ticks.

answered Oct 07 '19 at 09:26

stathisv

489
2
7

score 0 · Answer 2 · answered Oct 12 '19 at 20:27

To answer your doubts - yes, this code will work. From the technical point of view, the RTOS code itself doesn't really care about the function which takes or releases the mutex, it's the task that executes the code that matters (or more specifically the context, as we also include interrupts).

In fact, a while ago (some FreeRTOS 7 version I think?) they've introduced an additional check in the function releasing the mutex which compares the task releasing the mutex to the task that holds the mutex. If it's not the same one, it actually fails an assert which is effectively an endless loop stopping your task from continuing further so that you can notice and fix the issue (there's extensive comments around asserts which help you diagnose the issue). It's done this way as mutexes are used to guard resources - think SD card access, display access and similar - and taking a mutex from one task and releasing it from another goes against this whole idea, or at least points to smelly code. If you need to do something like this, you likely wanted to use a semaphore instead.

Now as for the second part of your question - whether that's a good practice to do that - I'd say it depends how complicated you make it, but generally I consider it questionable. In general the code operating on a mutex looks something like this:

if(xSemaphoreTake(mutex, waitTime) == pdTRUE)
{
  doResourceOperation();
  xSemaphoreGive(mutex);
}

It's simple, it's easy to understand as that's how most are used to writing code like this. It pretty much avoids whole class of problems with this mutex which may arise if you start complicating code taking and releasing it ("Why isn't it released?", "Who holds this mutex?", "Am I in a deadlock?"). These kinds of problems tend to be hard to debug and sometimes even hard to fix because it may involve rearranging some parts of the code.

To give a general advice - keep it simple. Seeing some weird things being done around a mutex often means there's some questionable things going there. Possible some nasty deadlocks or race conditions. As in your example, instead of trying to take the mutex every 50ms forever until it succeeds, just wait forever by specifying portMAX_DELAY delay time for xSemaphoreTake and put it inside the same function that uses the resource and releases the mutex.

Use an external function to take a mutex within a task

2 Answers2