From 7e45aee8a564ea588bca04ad75841cd63f79efc4 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 17 Nov 2017 14:13:38 -0600 Subject: [PATCH] Fixed mutex assert in armcc fopen and related memory leak armcc fopen allocated a mutex using the retargeted system-level _mutex_initialize function. Interestingly, malloc also uses this same _mutex_initialization function, which prevents a full solution relying on malloc. The solution previously implemented involved using the rtx mutex pool for the first 8 mutexes, then falling back on malloc. The previous implementation relied on osMutexNew returning an error on out-of-memory. An unrelated change causes osMutexNew to instead assert (except for release mode). This meant if you exceed 8 system- level mutexes in armcc you will hit an assert. Since the filesystem code can call fopen an unlimited number of times, this is a problem. Solution is to keep track of which static mutexes we've allocated, so we know before calling osMutexNew if we need to call malloc. Also _mutex_free never deallocated the malloced mutexes, which would cause fopen to leak memory. --- rtos/TARGET_CORTEX/mbed_boot.c | 44 ++++++++++++++++++-- rtos/TARGET_CORTEX/rtx5/RTX/Source/rtx_lib.c | 12 ++++-- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/rtos/TARGET_CORTEX/mbed_boot.c b/rtos/TARGET_CORTEX/mbed_boot.c index 646f307d0df..46bc1f1b115 100644 --- a/rtos/TARGET_CORTEX/mbed_boot.c +++ b/rtos/TARGET_CORTEX/mbed_boot.c @@ -168,6 +168,7 @@ #include "cmsis_os2.h" #include "mbed_toolchain.h" #include "mbed_error.h" +#include "mbed_critical.h" #if defined(__IAR_SYSTEMS_ICC__ ) && (__VER__ >= 8000000) #include #endif @@ -423,6 +424,7 @@ void __rt_entry (void) { } typedef void *mutex; +mutex _static_mutexes[OS_MUTEX_NUM] = {NULL}; /* ARM toolchain requires dynamically created mutexes to enforce thread safety. There's up to 8 static mutexes, protecting atexit, signalinit, stdin, stdout, stderr, stream_list, @@ -441,9 +443,23 @@ int _mutex_initialize(mutex *m) attr.name = "ARM toolchain mutex"; attr.attr_bits = osMutexRecursive | osMutexPrioInherit | osMutexRobust; - *m = osMutexNew(&attr); - if (*m != NULL) { - return 1; + mutex *slot = NULL; + core_util_critical_section_enter(); + for (int i = 0; i < OS_MUTEX_NUM; i++) { + if (_static_mutexes[i] == NULL) { + _static_mutexes[i] = (mutex)-1; // dummy value to reserve slot + slot = &_static_mutexes[i]; + break; + } + } + core_util_critical_section_exit(); + + if (slot != NULL) { + *m = osMutexNew(&attr); + *slot = *m; + if (*m != NULL) { + return 1; + } } /* Mutex pool exhausted, try using HEAP */ @@ -463,6 +479,28 @@ int _mutex_initialize(mutex *m) return 1; } +void _mutex_free(mutex *m) { + mutex *slot = NULL; + core_util_critical_section_enter(); + for (int i = 0; i < OS_MUTEX_NUM; i++) { + if (_static_mutexes[i] == *m) { + slot = &_static_mutexes[i]; + break; + } + } + core_util_critical_section_exit(); + + osMutexDelete(*m); + + // if no slot reserved for mutex, must have been dynamically allocated + if (!slot) { + free(m); + } else { + *slot = NULL; + } + +} + #endif /* ARMC */ #elif defined (__GNUC__) /******************** GCC ********************/ diff --git a/rtos/TARGET_CORTEX/rtx5/RTX/Source/rtx_lib.c b/rtos/TARGET_CORTEX/rtx5/RTX/Source/rtx_lib.c index 26325c507b9..70b18e917a8 100644 --- a/rtos/TARGET_CORTEX/rtx5/RTX/Source/rtx_lib.c +++ b/rtos/TARGET_CORTEX/rtx5/RTX/Source/rtx_lib.c @@ -609,27 +609,33 @@ __WEAK int _mutex_initialize(mutex *m) { } // Acquire mutex +#if !defined(__ARMCC_VERSION) || __ARMCC_VERSION < 6010050 __USED +#endif void _mutex_acquire(mutex *m); -void _mutex_acquire(mutex *m) { +__WEAK void _mutex_acquire(mutex *m) { if (os_kernel_is_active()) { osMutexAcquire(*m, osWaitForever); } } // Release mutex +#if !defined(__ARMCC_VERSION) || __ARMCC_VERSION < 6010050 __USED +#endif void _mutex_release(mutex *m); -void _mutex_release(mutex *m) { +__WEAK void _mutex_release(mutex *m) { if (os_kernel_is_active()) { osMutexRelease(*m); } } // Free mutex +#if !defined(__ARMCC_VERSION) || __ARMCC_VERSION < 6010050 __USED +#endif void _mutex_free(mutex *m); -void _mutex_free(mutex *m) { +__WEAK void _mutex_free(mutex *m) { osMutexDelete(*m); }