[PATCH v2] score: PR788: Add INTERNAL_ERROR_RESOURCE_IN_USE
Gedare Bloom
gedare at rtems.org
Mon Mar 31 14:15:00 UTC 2014
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.huber at 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 at .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
> + * <rtems at 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 at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel
More information about the devel
mailing list