From 90dda9357ecbcc1aa53ee207a650d07a4192ac2f Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Wed, 31 May 2017 10:55:37 -0600 Subject: [PATCH] pthread robust mutexes: Fix memmory trashing problem: the main task may also use mutexes; need to check thread type before accessing pthread-specific mutex data structures. Problem noted by Jussi Kivilinna. --- configs/sim/ostest/defconfig | 30 +++++++- sched/pthread/pthread_mutex.c | 133 +++++++++++++++++++++++----------- 2 files changed, 117 insertions(+), 46 deletions(-) diff --git a/configs/sim/ostest/defconfig b/configs/sim/ostest/defconfig index d13dc5b991..374b538fc4 100644 --- a/configs/sim/ostest/defconfig +++ b/configs/sim/ostest/defconfig @@ -28,6 +28,7 @@ CONFIG_BUILD_FLAT=y # CONFIG_MOTOROLA_SREC is not set # CONFIG_RAW_BINARY is not set # CONFIG_UBOOT_UIMAGE is not set +# CONFIG_DFU_BINARY is not set # # Customize Header Files @@ -77,11 +78,11 @@ CONFIG_HOST_X86_64=y CONFIG_SIM_X8664_SYSTEMV=y # CONFIG_SIM_X8664_MICROSOFT is not set # CONFIG_SIM_WALLTIME is not set -CONFIG_SIM_NET_HOST_ROUTE=y -# CONFIG_SIM_NET_BRIDGE is not set # CONFIG_SIM_FRAMEBUFFER is not set # CONFIG_SIM_SPIFLASH is not set # CONFIG_SIM_QSPIFLASH is not set +# CONFIG_ARCH_TOOLCHAIN_IAR is not set +# CONFIG_ARCH_TOOLCHAIN_GNU is not set # # Architecture Options @@ -102,6 +103,7 @@ CONFIG_ARCH_HAVE_MULTICPU=y # CONFIG_ARCH_HAVE_EXTCLK is not set CONFIG_ARCH_HAVE_POWEROFF=y # CONFIG_ARCH_HAVE_RESET is not set +# CONFIG_ARCH_HAVE_RTC_SUBSECONDS is not set # CONFIG_ARCH_STACKDUMP is not set # CONFIG_ENDIAN_BIG is not set # CONFIG_ARCH_IDLE_CUSTOM is not set @@ -204,6 +206,8 @@ CONFIG_SCHED_WAITPID=y # CONFIG_PTHREAD_MUTEX_TYPES=y CONFIG_PTHREAD_MUTEX_ROBUST=y +# CONFIG_PTHREAD_MUTEX_UNSAFE is not set +# CONFIG_PTHREAD_MUTEX_BOTH is not set CONFIG_NPTHREAD_KEYS=4 # CONFIG_PTHREAD_CLEANUP is not set # CONFIG_CANCELLATION_POINTS is not set @@ -368,6 +372,7 @@ CONFIG_SERIAL_CONSOLE=y # CONFIG_PSEUDOTERM is not set # CONFIG_USBDEV is not set # CONFIG_USBHOST is not set +# CONFIG_USBMISC is not set # CONFIG_HAVE_USBTRACE is not set # CONFIG_DRIVERS_WIRELESS is not set # CONFIG_DRIVERS_CONTACTLESS is not set @@ -376,7 +381,9 @@ CONFIG_SERIAL_CONSOLE=y # System Logging # # CONFIG_ARCH_SYSLOG is not set +CONFIG_SYSLOG_WRITE=y # CONFIG_RAMLOG is not set +# CONFIG_SYSLOG_BUFFER is not set # CONFIG_SYSLOG_INTBUFFER is not set # CONFIG_SYSLOG_TIMESTAMP is not set CONFIG_SYSLOG_SERIAL_CONSOLE=y @@ -437,6 +444,11 @@ CONFIG_MM_REGIONS=1 # CONFIG_ARCH_HAVE_HEAP2 is not set # CONFIG_GRAN is not set +# +# Common I/O Buffer Support +# +# CONFIG_MM_IOB is not set + # # Audio Support # @@ -445,6 +457,7 @@ CONFIG_MM_REGIONS=1 # # Wireless Support # +# CONFIG_WIRELESS is not set # # Binary Loader @@ -594,7 +607,6 @@ CONFIG_LIB_SENDFILE_BUFSIZE=512 # CONFIG_EXAMPLES_MM is not set # CONFIG_EXAMPLES_MODBUS is not set # CONFIG_EXAMPLES_MOUNT is not set -# CONFIG_EXAMPLES_NRF24L01TERM is not set # CONFIG_EXAMPLES_NSH is not set # CONFIG_EXAMPLES_NULL is not set # CONFIG_EXAMPLES_NX is not set @@ -630,6 +642,7 @@ CONFIG_EXAMPLES_OSTEST_WAITRESULT=y # CONFIG_EXAMPLES_TOUCHSCREEN is not set # CONFIG_EXAMPLES_WATCHDOG is not set # CONFIG_EXAMPLES_WEBSERVER is not set +# CONFIG_EXAMPLES_XBC_TEST is not set # # File System Utilities @@ -700,3 +713,14 @@ CONFIG_EXAMPLES_OSTEST_WAITRESULT=y # CONFIG_SYSTEM_UBLOXMODEM is not set # CONFIG_SYSTEM_VI is not set # CONFIG_SYSTEM_ZMODEM is not set + +# +# Wireless Libraries and NSH Add-Ons +# + +# +# IEEE 802.15.4 applications +# +# CONFIG_IEEE802154_LIBMAC is not set +# CONFIG_IEEE802154_LIBUTILS is not set +# CONFIG_IEEE802154_I8SAK is not set diff --git a/sched/pthread/pthread_mutex.c b/sched/pthread/pthread_mutex.c index 13dde1a53d..8b6566b8a2 100644 --- a/sched/pthread/pthread_mutex.c +++ b/sched/pthread/pthread_mutex.c @@ -58,7 +58,7 @@ * Name: pthread_mutex_add * * Description: - * Add the mutex to the list of mutexes held by this thread. + * Add the mutex to the list of mutexes held by this pthread. * * Parameters: * mutex - The mux to be locked @@ -70,17 +70,91 @@ static void pthread_mutex_add(FAR struct pthread_mutex_s *mutex) { - FAR struct pthread_tcb_s *rtcb = (FAR struct pthread_tcb_s *)this_task(); - irqstate_t flags; + FAR struct tcb_s *rtcb = this_task(); DEBUGASSERT(mutex->flink == NULL); - /* Add the mutex to the list of mutexes held by this task */ + /* Check if this is a pthread. The main thread may also lock and unlock + * mutexes. The main thread, however, does not participate in the mutex + * consistency logic. Presumably, when the main thread exits, all of the + * child pthreads will also terminate. + * + * REVISIT: NuttX does not support that behavior at present; child pthreads + * will persist after the main thread exits. + */ - flags = enter_critical_section(); - mutex->flink = rtcb->mhead; - rtcb->mhead = mutex; - leave_critical_section(flags); + if ((rtcb->flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_PTHREAD) + { + FAR struct pthread_tcb_s *ptcb = (FAR struct pthread_tcb_s *)rtcb; + irqstate_t flags; + + /* Add the mutex to the list of mutexes held by this pthread */ + + flags = enter_critical_section(); + mutex->flink = ptcb->mhead; + ptcb->mhead = mutex; + leave_critical_section(flags); + } +} + +/**************************************************************************** + * Name: pthread_mutex_remove + * + * Description: + * Remove the mutex to the list of mutexes held by this pthread. + * + * Parameters: + * mutex - The mux to be locked + * + * Return Value: + * None + * + ****************************************************************************/ + +static void pthread_mutex_remove(FAR struct pthread_mutex_s *mutex) +{ + FAR struct tcb_s *rtcb = this_task(); + + DEBUGASSERT(mutex->flink == NULL); + + /* Check if this is a pthread. The main thread may also lock and unlock + * mutexes. The main thread, however, does not participate in the mutex + * consistency logic. + */ + + if ((rtcb->flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_PTHREAD) + { + FAR struct pthread_tcb_s *ptcb = (FAR struct pthread_tcb_s *)rtcb; + FAR struct pthread_mutex_s *curr; + FAR struct pthread_mutex_s *prev; + irqstate_t flags; + + flags = enter_critical_section(); + + /* Remove the mutex from the list of mutexes held by this task */ + + for (prev = NULL, curr = ptcb->mhead; + curr != NULL && curr != mutex; + prev = curr, curr = curr->flink); + + DEBUGASSERT(curr == mutex); + + /* Remove the mutex from the list. prev == NULL means that the mutex + * to be removed is at the head of the list. + */ + + if (prev == NULL) + { + ptcb->mhead = mutex->flink; + } + else + { + prev->flink = mutex->flink; + } + + mutex->flink = NULL; + leave_critical_section(flags); + } } /**************************************************************************** @@ -233,8 +307,6 @@ int pthread_mutex_trytake(FAR struct pthread_mutex_s *mutex) int pthread_mutex_give(FAR struct pthread_mutex_s *mutex) { - FAR struct pthread_mutex_s *curr; - FAR struct pthread_mutex_s *prev; int ret = EINVAL; /* Verify input parameters */ @@ -242,34 +314,9 @@ int pthread_mutex_give(FAR struct pthread_mutex_s *mutex) DEBUGASSERT(mutex != NULL); if (mutex != NULL) { - FAR struct pthread_tcb_s *rtcb = (FAR struct pthread_tcb_s *)this_task(); - irqstate_t flags; - - flags = enter_critical_section(); - /* Remove the mutex from the list of mutexes held by this task */ - for (prev = NULL, curr = rtcb->mhead; - curr != NULL && curr != mutex; - prev = curr, curr = curr->flink); - - DEBUGASSERT(curr == mutex); - - /* Remove the mutex from the list. prev == NULL means that the mutex - * to be removed is at the head of the list. - */ - - if (prev == NULL) - { - rtcb->mhead = mutex->flink; - } - else - { - prev->flink = mutex->flink; - } - - mutex->flink = NULL; - leave_critical_section(flags); + pthread_mutex_remove(mutex); /* Now release the underlying semaphore */ @@ -300,7 +347,7 @@ int pthread_mutex_give(FAR struct pthread_mutex_s *mutex) #ifdef CONFIG_CANCELLATION_POINTS uint16_t pthread_disable_cancel(void) { - FAR struct pthread_tcb_s *tcb = (FAR struct pthread_tcb_s *)this_task(); + FAR struct tcb_s *tcb = this_task(); irqstate_t flags; uint16_t old; @@ -309,15 +356,15 @@ uint16_t pthread_disable_cancel(void) */ flags = enter_critical_section(); - old = tcb->cmn.flags & (TCB_FLAG_CANCEL_PENDING | TCB_FLAG_NONCANCELABLE); - tcb->cmn.flags &= ~(TCB_FLAG_CANCEL_PENDING | TCB_FLAG_NONCANCELABLE); + old = tcb->flags & (TCB_FLAG_CANCEL_PENDING | TCB_FLAG_NONCANCELABLE); + tcb->flags &= ~(TCB_FLAG_CANCEL_PENDING | TCB_FLAG_NONCANCELABLE); leave_critical_section(flags); return old; } void pthread_enable_cancel(uint16_t cancelflags) { - FAR struct pthread_tcb_s *tcb = (FAR struct pthread_tcb_s *)this_task(); + FAR struct tcb_s *tcb = this_task(); irqstate_t flags; /* We need perform the following operations from within a critical section @@ -325,7 +372,7 @@ void pthread_enable_cancel(uint16_t cancelflags) */ flags = enter_critical_section(); - tcb->cmn.flags |= cancelflags; + tcb->flags |= cancelflags; /* What should we do if there is a pending cancellation? * @@ -337,8 +384,8 @@ void pthread_enable_cancel(uint16_t cancelflags) * then we need to terminate now by simply calling pthread_exit(). */ - if ((tcb->cmn.flags & TCB_FLAG_CANCEL_DEFERRED) == 0 && - (tcb->cmn.flags & TCB_FLAG_CANCEL_PENDING) != 0) + if ((tcb->flags & TCB_FLAG_CANCEL_DEFERRED) == 0 && + (tcb->flags & TCB_FLAG_CANCEL_PENDING) != 0) { pthread_exit(NULL); }