bcc: rtems-users Sebastian,
Any thought on whether we can resolve this problem of threads holding resources by way of something similar to the discussion about solving the "strict order mutex" / nested mutex acquire bug https://www.rtems.org/bugzilla/show_bug.cgi?id=2124 For now I'm good with the fatal error, it is better than silently leaking the resource for sure. Gedare On Mon, Mar 31, 2014 at 7:51 AM, Sebastian Huber <sebastian.hu...@embedded-brains.de> wrote: > Issue a fatal error in case a thread is deleted which still owns > resources (e.g. a binary semaphore with priority inheritance or ceiling > protocol). The resource count must be checked quite late since RTEMS > task variable destructors, POSIX key destructors, POSIX cleanup handler, > the Newlib thread termination extension or other thread termination > extensions may release resources. In this context it would be quite > difficult to return an error status to the caller. > > An alternative would be to place threads with a non-zero resource count > not on the zombie chain. Thus we have a resource leak instead of a > fatal error. The terminator thread can see this error if we return an > RTEMS_RESOURCE_IN_USE status for the rtems_task_delete() for example. > --- > cpukit/sapi/src/interrtext.c | 3 +- > cpukit/score/include/rtems/score/interr.h | 3 +- > cpukit/score/src/threadrestart.c | 8 +++++ > testsuites/sptests/Makefile.am | 1 + > testsuites/sptests/configure.ac | 1 + > testsuites/sptests/spfatal28/Makefile.am | 19 +++++++++++++ > testsuites/sptests/spfatal28/spfatal28.doc | 12 ++++++++ > testsuites/sptests/spfatal28/spfatal28.scn | 3 ++ > testsuites/sptests/spfatal28/testcase.h | 39 > +++++++++++++++++++++++++++ > testsuites/sptests/spinternalerror02/init.c | 2 +- > testsuites/sptests/spsem01/init.c | 6 ++-- > testsuites/sptests/spsem01/spsem01.scn | 9 +++--- > testsuites/sptests/spsem02/init.c | 6 ++-- > testsuites/sptests/spsem02/spsem02.scn | 7 ++--- > 14 files changed, 101 insertions(+), 18 deletions(-) > create mode 100644 testsuites/sptests/spfatal28/Makefile.am > create mode 100644 testsuites/sptests/spfatal28/spfatal28.doc > create mode 100644 testsuites/sptests/spfatal28/spfatal28.scn > create mode 100644 testsuites/sptests/spfatal28/testcase.h > > diff --git a/cpukit/sapi/src/interrtext.c b/cpukit/sapi/src/interrtext.c > index a18610c..e08414b 100644 > --- a/cpukit/sapi/src/interrtext.c > +++ b/cpukit/sapi/src/interrtext.c > @@ -51,7 +51,8 @@ static const char *const internal_error_text[] = { > "INTERNAL_ERROR_GXX_KEY_ADD_FAILED", > "INTERNAL_ERROR_GXX_MUTEX_INIT_FAILED", > "INTERNAL_ERROR_NO_MEMORY_FOR_HEAP", > - "INTERNAL_ERROR_CPU_ISR_INSTALL_VECTOR" > + "INTERNAL_ERROR_CPU_ISR_INSTALL_VECTOR", > + "INTERNAL_ERROR_RESOURCE_IN_USE" > }; > > const char *rtems_internal_error_text( rtems_fatal_code error ) > diff --git a/cpukit/score/include/rtems/score/interr.h > b/cpukit/score/include/rtems/score/interr.h > index 8c647f5..2100c13 100644 > --- a/cpukit/score/include/rtems/score/interr.h > +++ b/cpukit/score/include/rtems/score/interr.h > @@ -147,7 +147,8 @@ typedef enum { > INTERNAL_ERROR_GXX_KEY_ADD_FAILED, > INTERNAL_ERROR_GXX_MUTEX_INIT_FAILED, > INTERNAL_ERROR_NO_MEMORY_FOR_HEAP, > - INTERNAL_ERROR_CPU_ISR_INSTALL_VECTOR > + INTERNAL_ERROR_CPU_ISR_INSTALL_VECTOR, > + INTERNAL_ERROR_RESOURCE_IN_USE > } Internal_errors_Core_list; > > typedef uint32_t Internal_errors_t; > diff --git a/cpukit/score/src/threadrestart.c > b/cpukit/score/src/threadrestart.c > index 4b83ede..02371ac 100644 > --- a/cpukit/score/src/threadrestart.c > +++ b/cpukit/score/src/threadrestart.c > @@ -47,6 +47,14 @@ static void _Thread_Make_zombie( Thread_Control > *the_thread ) > ISR_lock_Context lock_context; > Thread_Zombie_control *zombies = &_Thread_Zombies; > > + if ( the_thread->resource_count != 0 ) { > + _Terminate( > + INTERNAL_ERROR_CORE, > + false, > + INTERNAL_ERROR_RESOURCE_IN_USE > + ); > + } > + > _Thread_Set_state( the_thread, STATES_ZOMBIE ); > _Thread_queue_Extract_with_proxy( the_thread ); > _Watchdog_Remove( &the_thread->Timer ); > diff --git a/testsuites/sptests/Makefile.am b/testsuites/sptests/Makefile.am > index d47f6f7..c344ce0 100644 > --- a/testsuites/sptests/Makefile.am > +++ b/testsuites/sptests/Makefile.am > @@ -33,6 +33,7 @@ SUBDIRS = \ > spsignal_err01 spport_err01 spmsgq_err01 spmsgq_err02 spsem_err01 \ > spsem_err02 sptask_err01 spevent_err03 sptask_err03 sptask_err02 \ > sptask_err04 spclock_err01 > +SUBDIRS += spfatal28 > SUBDIRS += spthreadlife01 > SUBDIRS += spprofiling01 > SUBDIRS += spcache01 > diff --git a/testsuites/sptests/configure.ac b/testsuites/sptests/configure.ac > index 4809bdc..3f2f952 100644 > --- a/testsuites/sptests/configure.ac > +++ b/testsuites/sptests/configure.ac > @@ -36,6 +36,7 @@ AM_CONDITIONAL(HAS_CPUSET,test > x"${ac_cv_header_sys_cpuset_h}" = x"yes") > > # Explicitly list all Makefiles here > AC_CONFIG_FILES([Makefile > +spfatal28/Makefile > spthreadlife01/Makefile > spprofiling01/Makefile > spcache01/Makefile > diff --git a/testsuites/sptests/spfatal28/Makefile.am > b/testsuites/sptests/spfatal28/Makefile.am > new file mode 100644 > index 0000000..a20d936 > --- /dev/null > +++ b/testsuites/sptests/spfatal28/Makefile.am > @@ -0,0 +1,19 @@ > +rtems_tests_PROGRAMS = spfatal28 > +spfatal28_SOURCES = ../spfatal_support/init.c ../spfatal_support/system.h > testcase.h > + > +dist_rtems_tests_DATA = spfatal28.scn spfatal28.doc > + > +include $(RTEMS_ROOT)/make/custom/@RTEMS_BSP@.cfg > +include $(top_srcdir)/../automake/compile.am > +include $(top_srcdir)/../automake/leaf.am > + > +AM_CPPFLAGS += -I$(top_srcdir)/../support/include > + > +LINK_OBJS = $(spfatal28_OBJECTS) > +LINK_LIBS = $(spfatal28_LDLIBS) > + > +spfatal28$(EXEEXT): $(spfatal28_OBJECTS) $(spfatal28_DEPENDENCIES) > + @rm -f spfatal28$(EXEEXT) > + $(make-exe) > + > +include $(top_srcdir)/../automake/local.am > diff --git a/testsuites/sptests/spfatal28/spfatal28.doc > b/testsuites/sptests/spfatal28/spfatal28.doc > new file mode 100644 > index 0000000..7514dd9 > --- /dev/null > +++ b/testsuites/sptests/spfatal28/spfatal28.doc > @@ -0,0 +1,12 @@ > +This file describes the directives and concepts tested by this test set. > + > +test set name: spfatal28 > + > +directives: > + > + - rtems_task_delete() > + > +concepts: > + > + - Ensure that deletion of a task with a semaphore in use leads to a fatal > + error. > diff --git a/testsuites/sptests/spfatal28/spfatal28.scn > b/testsuites/sptests/spfatal28/spfatal28.scn > new file mode 100644 > index 0000000..6fcae10 > --- /dev/null > +++ b/testsuites/sptests/spfatal28/spfatal28.scn > @@ -0,0 +1,3 @@ > +*** BEGIN OF TEST SPFATAL 28 *** > +Fatal error (delete a task with a semaphore in use) hit > +*** END OF TEST SPFATAL 28 *** > diff --git a/testsuites/sptests/spfatal28/testcase.h > b/testsuites/sptests/spfatal28/testcase.h > new file mode 100644 > index 0000000..fc72100 > --- /dev/null > +++ b/testsuites/sptests/spfatal28/testcase.h > @@ -0,0 +1,39 @@ > +/* > + * Copyright (c) 2014 embedded brains GmbH. All rights reserved. > + * > + * embedded brains GmbH > + * Dornierstr. 4 > + * 82178 Puchheim > + * Germany > + * <rt...@embedded-brains.de> > + * > + * The license and distribution terms for this file may be > + * found in the file LICENSE in this distribution or at > + * http://www.rtems.org/license/LICENSE. > + */ > + > +#define FATAL_ERROR_TEST_NAME "28" > +#define FATAL_ERROR_DESCRIPTION "delete a task with a semaphore in > use" > +#define FATAL_ERROR_EXPECTED_SOURCE INTERNAL_ERROR_CORE > +#define FATAL_ERROR_EXPECTED_IS_INTERNAL FALSE > +#define FATAL_ERROR_EXPECTED_ERROR INTERNAL_ERROR_RESOURCE_IN_USE > + > +#define CONFIGURE_MAXIMUM_SEMAPHORES 1 > + > +void force_error() > +{ > + rtems_status_code sc; > + rtems_id id; > + > + sc = rtems_semaphore_create( > + rtems_build_name('S', 'E', 'M', 'A'), > + 0, > + RTEMS_BINARY_SEMAPHORE | RTEMS_PRIORITY | RTEMS_INHERIT_PRIORITY, > + 0, > + &id > + ); > + rtems_test_assert(sc == RTEMS_SUCCESSFUL); > + > + sc = rtems_task_delete(RTEMS_SELF); > + rtems_test_assert(sc == RTEMS_SUCCESSFUL); > +} > diff --git a/testsuites/sptests/spinternalerror02/init.c > b/testsuites/sptests/spinternalerror02/init.c > index 340a9e3..3302a2c 100644 > --- a/testsuites/sptests/spinternalerror02/init.c > +++ b/testsuites/sptests/spinternalerror02/init.c > @@ -35,7 +35,7 @@ static void test_internal_error_text(void) > puts( text ); > } while ( text != text_last ); > > - rtems_test_assert( error - 3 == INTERNAL_ERROR_CPU_ISR_INSTALL_VECTOR ); > + rtems_test_assert( error - 3 == INTERNAL_ERROR_RESOURCE_IN_USE ); > } > > static void test_fatal_source_text(void) > diff --git a/testsuites/sptests/spsem01/init.c > b/testsuites/sptests/spsem01/init.c > index e950890..924f4c0 100644 > --- a/testsuites/sptests/spsem01/init.c > +++ b/testsuites/sptests/spsem01/init.c > @@ -150,8 +150,8 @@ rtems_task Task02(rtems_task_argument ignored) > directive_failed( status, " rtems_semaphore_obtain S1"); > printf("TA02: priority %d, holding S1\n", getprio()); > > - printf("TA02: exiting\n"); > - status = rtems_task_delete( RTEMS_SELF); > - directive_failed( status, "rtems_task_delete TA02"); > + printf("TA02: suspending\n"); > + status = rtems_task_suspend( RTEMS_SELF); > + directive_failed( status, "rtems_task_suspend TA02"); > } > > diff --git a/testsuites/sptests/spsem01/spsem01.scn > b/testsuites/sptests/spsem01/spsem01.scn > index 8b0a59a..35e6b94 100644 > --- a/testsuites/sptests/spsem01/spsem01.scn > +++ b/testsuites/sptests/spsem01/spsem01.scn > @@ -1,4 +1,4 @@ > -*** TEST SEM01 *** > +*** BEGIN OF TEST SPSEM 1 *** > init: S0 created > init: S1 created > init: TA01 created with priority 36 > @@ -7,10 +7,9 @@ TA01: started with priority 36 > TA01: priority 36, holding S0 > TA01: priority 36, holding S0, S1 > TA02: started with priority 34 > +TA01: priority 34, holding S0 > TA02: priority 34, holding S1 > -TA02: exiting > -TA01: priority 36, holding S0 > +TA02: suspending > TA01: priority 36 > TA01: exiting > -*** END OF SEM01 *** > - > +*** END OF TEST SPSEM 1 *** > diff --git a/testsuites/sptests/spsem02/init.c > b/testsuites/sptests/spsem02/init.c > index c88e9ad..d5fb47b 100644 > --- a/testsuites/sptests/spsem02/init.c > +++ b/testsuites/sptests/spsem02/init.c > @@ -168,9 +168,9 @@ rtems_task Task02(rtems_task_argument ignored) > directive_failed( status, " rtems_semaphore_obtain S1"); > printf("TA02: priority %d, holding S1\n", getprio()); > > - printf("TA02: exiting\n"); > - status = rtems_task_delete( RTEMS_SELF); > - directive_failed( status, "rtems_task_delete TA02"); > + printf("TA02: suspending\n"); > + status = rtems_task_suspend( RTEMS_SELF); > + directive_failed( status, "rtems_task_suspend TA02"); > } > > /* Task03 starts with priority 32 */ > diff --git a/testsuites/sptests/spsem02/spsem02.scn > b/testsuites/sptests/spsem02/spsem02.scn > index 5b8d03d..ab8c7ce 100644 > --- a/testsuites/sptests/spsem02/spsem02.scn > +++ b/testsuites/sptests/spsem02/spsem02.scn > @@ -1,4 +1,4 @@ > -*** TEST SEM02 *** > +*** BEGIN OF TEST SPSEM 2 *** > init: S0 created > init: S1 created > init: TA01 created with priority 36 > @@ -15,8 +15,7 @@ TA03: priority 32, holding S0 > TA03: priority 32 > TA03: exiting > TA02: priority 34, holding S1 > -TA02: exiting > +TA02: suspending > TA01: priority 36 > TA01: exiting > -*** END OF SEM02 *** > - > +*** END OF TEST SPSEM 2 *** > -- > 1.7.7 > > _______________________________________________ > rtems-devel mailing list > rtems-devel@rtems.org > http://www.rtems.org/mailman/listinfo/rtems-devel _______________________________________________ rtems-devel mailing list rtems-devel@rtems.org http://www.rtems.org/mailman/listinfo/rtems-devel
