Thread safe memory allocation

parmi wrote on Friday, August 09, 2019:

I’m using C ++ with FreeRTOS 10.0.1, so I use the operators new, delete, new[], delete[], new MyClass(), vectors, new string(), string concatenation (which internally can cause a reallocation of the memory used by the string).

But I suspect that all these operations are not thread safe, can anyone confirm me?

If my suspicions are true, can I make these operations thread safe? for example by overriding “malloc (…)” and “free (…)”.

void *malloc(size_t size)
{
	return (pvPortMalloc(size));
}
void free(void *ptr)
{
	vPortFree(ptr);
}

I read a lot of discussions about it, but I still preferred to open this discussion.
I also discovered “configUSE_NEWLIB_REENTRANT”, what is it? it can be useful?

richard_damon wrote on Friday, August 09, 2019:

By default, new/delete are often not thread safe in a FreeRTOS app because the basic malloc and free are not thread safe.

Some embedded libraries have hooks that can be defined to make these functions thread safe. For example, newlib, if it was compiled properly, will use the functions __malloc_lock() and __malloc_unlock() to allow you to protect the library from re-entrancy. I often define them with a call to vTaskSuspendAll() / vTaskResumeAll() to provide that protection.

You also may be able to replace them with alternate functions that do what you want. The one issue here is you need to dig into the library, as sometimes malloc/free are just thin wrappers around something deeper internally, and parts of the library may just call those internals, so THOSE are what willl need to be replaced.

If you are only worried about fixing new/delete, then you can over ride the implementation of those to use a thread safe allocator, (be sure to get new[] and delete{} too), but this will not fix usage of malloc and free.

configUSE_NEWLIB_REENTRANT is a flag that creates a structure for every thread that contains a bunch of internal state used by some functions in newlib, depending on some library options, this can be fairly small or quite big (as it can contain the File IO Buffers). Programs designed for small systems often don’t use the parts that need that structure on a per-task basis, as they don’t use that functionality. If you do use it, you need to define that to 1 to get the needed thread safety, or manually protect those parts of the library from reentrancy.

parmi wrote on Friday, August 09, 2019:

Thank you so much for the answer Richard!

So I understand that “configUSE_NEWLIB_REENTRANT” would not solve my problem.
I’m thinking about overriding the operator new, delete, new[] and delete[].

void* operator new( size_t size )
{
    if (xTaskGetSchedulerState() == taskSCHEDULER_NOT_STARTED)
	{
        //what should I do???????
	}
    return pvPortMalloc( size );
}

void* operator new[]( size_t size )
{
    return pvPortMalloc(size);
}

void operator delete( void * ptr )
{
    vPortFree ( ptr );
}

void operator delete[]( void * ptr )
{
    vPortFree ( ptr );
}

But I’m not sure this is the best solution.
For example, the “string” class can allocate memory dynamically when concatenating a string.

string my_string = string ("Hello");

/* a memory allocation can be performed internally, which operator uses the "string" class? "new" or "malloc(...)"? */
my_string + = "world!"

richard_damon wrote on Saturday, August 10, 2019:

OVerriding operatior new/delete that way will replace them, but only for things that use new/delete. string probably does, but I am not sure it is promised by the standard.

The better option in my mind is defining the functions

__malloc_lock() and __malloc_unlock()

if your implementation uses them (a quick check is to see if they appear in the linker map without you defining them), if it does then the proper definition will make the whole memory allocation system thread safe with no need to worry about if there is something getting arround the protections.

dnadler wrote on Tuesday, August 13, 2019:

Parminder you did not say what toolchain and what C runtime library you are using. The above discussion is appropriate if you are using newlib.

parmi wrote on Monday, August 19, 2019:

I tried to define the malloc_lock and malloc_unlock functions, but the linker gives me an error.

main.cpp

extern "C" void __malloc_lock(struct _reent *REENT)
{
	vPortEnterCritical();
}

extern "C" void __malloc_unlock(struct _reent *REENT)
{
	vPortExitCritical();
}

Linking error:

1>Linking ../MyProject/...
1>c:/sysgcc/arm-eabi/bin/../lib/gcc/arm-eabi/7.2.0/../../../../arm-eabi/lib/thumb/cortex_m4\libc_nano.a(lib_a-mlock.o): In function `__malloc_lock':
1>q:\gnu\newlib-nano\gcc-arm-none-eabi-6-2017-q2-update\src\newlib\newlib\libc\stdlib\mlock.c(53): error : multiple definition of `__malloc_lock'
1>VisualGDB/Debug/main.o:C:\visual_gdb_workspace\MyProject\MyProject/main.cpp:130: first defined here
1>collect2.exe : error : ld returned 1 exit status

richard_damon wrote on Monday, August 19, 2019:

That sounds like your newlib was somehow build wrong or you system isn’t compatible with a right definition as the definitions in mlock are supposed to be weak, so are ok to be overriden by your copy. That probably needs to be taken up with the supplyer of your tools for the processor.

dnadler wrote on Monday, August 19, 2019:

While it should not be necessary, with gcc you can override the non-weak
implementations using the linker “wrap” option.
See example in code here (though different functions are wrapped):
http://www.nadler.com/embedded/newlibAndFreeRTOS.html
What processor and toolchain are you using?

On 8/19/2019 6:55 AM, Richard Damon wrote:

That sounds like your newlib was somehow build wrong or you system isn’t compatible with a right definition as the definitions in mlock are supposed to be weak, so are ok to be overriden by your copy. That probably needs to be taken up with the supplyer of your tools for the processor.

Thread safe memory allocation

Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/freertos/discussion/382005/
To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/


Dave Nadler, USA East Coast voice (978) 263-xxx, xxx@xxx.com, Skype
xxxx.xxxx

aggarg-aws wrote on Monday, August 19, 2019:

I cloned the newlib source and I do not see lock and unlock functions as weak in mlock.c:

void
__malloc_lock (ptr)
     struct _reent *ptr;
{
#ifndef __SINGLE_THREAD__
  __lock_acquire_recursive (__malloc_recursive_mutex);
#endif
}

void
__malloc_unlock (ptr)
     struct _reent *ptr;
{
#ifndef __SINGLE_THREAD__
  __lock_release_recursive (__malloc_recursive_mutex);
#endif
}

I may be looking in incorrect place though.

However, as Dave suggested, you can use GCC wrap feature. Pass the following flags to GCC:

-Wl,--wrap=__malloc_lock
-Wl,--wrap=__malloc_unlock

And then implement the following functions:

void __wrap___malloc_lock(struct _reent *r)
{
	vTaskSuspendAll();
}

void __wrap___malloc_unlock(struct _reent *r)
{
	xTaskResumeAll();
}

Thanks.

richard_damon wrote on Tuesday, August 20, 2019:

Looking closer at my projects, yes __malloc_lock/__malloc_unlock is not weak, but is generally in a library file, and linkers will generally allow the overriding of entries in a library with a entry in an object file, at long as no other symbol in that module in the library is needed. For some reason you are getting a fatal link error for doing this.

Also, I would not use vPortEnter/ExitCritical for this purpose, as that will disable interrrupts for a time that is too long for me. I use VTaskSuspend/ResumeAll for this to just stop the scheduler. You could also use a mutex to only block other tasks that need to use malloc (but watch out for the sequencing of creating the mutex with using malloc/free.

dnadler wrote on Tuesday, August 20, 2019:

I would not use vPortEnter/ExitCritical either, except in the STM case where the buffoons used malloc inside an ISR… That’s why vPortEnter/ExitCritical vs. VTaskSuspend/ResumeAll is conditionally compiled in my code grrrrr… ;-(

richard_damon wrote on Tuesday, August 20, 2019:

But you can’t call vPortEnterCritical inside an ISR, so it doesn’t solve the problem. (Now, I don’t use the STM code that uses malloc inside and ISR, I find a lot of the STM application support code desearving of replacement)

parmi wrote on Tuesday, August 20, 2019:

thank you all for your help.
I solved the problem, in my code (main.cpp) I had only overridden the function ‘malloc_lock(…)’, I had forgotten to override the function ‘malloc_unlock(…)’, and then for some reason the linker gave me that error.
When I overridden both the function the problem was solved, the functions are called correctly every time I do a “new”, “delete”, “new[]”, “delete[]”, “malloc(…)”, "free (…) ", concatenate a long string, push back into a vector.

Also now I’m using “vTaskSuspendAll()” and “xTaskResumeAll()” instead of “vPortEnterCritical()”, “vPortExitCritical()”.
main.cpp

extern "C" void __malloc_lock(struct _reent *REENT)
{	
	vTaskSuspendAll();
}

extern "C" void __malloc_unlock(struct _reent *REENT)
{
	xTaskResumeAll();
}

Another request, can I replace “malloc(…)” with “pvPortMalloc (…)” and “free(…)” with “vPortFree(…)”?

I’m developing with VisualStudio 2019 with VisualGDB 5.4 on a SMT32F429ZI
p.s. attached is the image of the toolchain I am using

richard_damon wrote on Tuesday, August 20, 2019:

If you define just one of the functions then the linker does still need the module from the library and then does have the duplicate symbol for the one you did define, and get the error.

You could in a similar manner redefine nalloc and free, but you will run into a couple of problems:

  1. newlib itself doesn’t call malloc and free, but instead something like __malloc_r() and __free_r()
  2. FreeRTOS doesn’t provide an equivalent to remalloc() because it doesn’t need it, but the standard library does, so it isn’t a complete memory solution.

Because of this, I tend to just use a modified version of heap3, that just translate calls to pvProtMalloc to calls to malloc, and adds the heap exhausted test

parmi wrote on Tuesday, August 20, 2019:

OK thanks, now I understand everything.
Where can I find this modified version of heap3? It is reliable?

richard_damon wrote on Tuesday, August 20, 2019:

I just made my own, starting with heap3.c, add the malloc_lock() and malloc_unlock() functions, and remove the calls to vTaskSuspendAll() and vTaskResumeAll() (since malloc/free will call them in lock/unlock).

dnadler wrote on Wednesday, August 21, 2019:

Parminder, there’s a complete heap_useNewlib.c here:
http://www.nadler.com/embedded/newlibAndFreeRTOS.html

Dear all,
For _malloc_lock and malloc_unlock instead of using vTaskSuspendAll and xTaskResumeAll could I use a mutex with priority inheritance (xSemaphoreTake and xSemaphoreGive)?

Best Regards

Yes, but you need to handle the issue of creating the mutex before you use it. I think there is a function called to give you a hook to do this. You will need to make the mutex statically or bypass the mutex while creating it.

yes I agree… during startup I could call a function inside heap.c file of freertos which create a mutex Calling xSemaphoreTake I don’t block all taskes. when I need to allocare memory from heap What do you think about it ?