Update TODO and some comments.

This commit is contained in:
Gregory Nutt 2016-12-08 10:24:40 -06:00
parent a1fbc2ad0d
commit ab43681f15
5 changed files with 54 additions and 21 deletions

41
TODO
View File

@ -108,9 +108,30 @@ o Task/Scheduler (sched/)
2. They run in supervisor mode (if applicable), and 2. They run in supervisor mode (if applicable), and
3. They do not obey any setup of PIC or address 3. They do not obey any setup of PIC or address
environments. Do they need to? environments. Do they need to?
4. In the case of task_delete() and pthread_cancel, these
callbacks will run on the thread of execution and address
context of the caller of task. That is very bad!
The fix for all of these issues it to have the callbacks The fix for all of these issues it to have the callbacks
run on the caller's thread (as with signal handlers). run on the caller's thread as is currently done with
signal handlers. Signals are delivered differently in
PROTECTED and KERNEL modes: The deliver is involes a
signal handling trampoline function in the user address
space and two signal handlers: One to call the signal
handler trampoline in user mode (SYS_signal_handler) and
on in with the signal handler trampoline to return to
supervisor mode (SYS_signal_handler_return)
The primary difference is in the location of the signal
handling trampoline:
- In PROTECTED mode, there is on a single user space blob
with a header at the beginning of the block (at a well-
known location. There is a pointer to the signal handler
trampoline function in that header.
- In the KERNEL mode, a special process signal handler
trampoline is used at a well-known location in every
process address space (ARCH_DATA_RESERVE->ar_sigtramp).
Status: Open Status: Open
Priority: Medium Low. This is an important change to some less Priority: Medium Low. This is an important change to some less
important interfaces. For the average user, these important interfaces. For the average user, these
@ -145,16 +166,26 @@ o Task/Scheduler (sched/)
Priority: Low Priority: Low
Title: REMOVE TASK_DELETE Title: REMOVE TASK_DELETE
Description: Need to remove or fix task delete. This interface is non- Description: Need to remove or fix task delete(). This interface is non-
standard and not safe. Arbitrary deleting tasks can cause standard and not safe. Arbitrary deleting tasks can cause
serious problems such as memory leaks. Better to remove it serious problems such as memory leaks and resources like
than to retain it as a latent bug. semaphores left in bad states.
Task/process exit callbacks registered via atexit() or
on_exit() will not work correctly with task_delete(): In
that case the callback would execute in the context the
caller of task_delete() cancel, not in the context of the
exiting task (or process).
Better to remove task_delete() than to retain it as a latent
bug.
Currently used within the OS and also part of the Currently used within the OS and also part of the
implementation of pthread_cancel() and task_restart() (which implementation of pthread_cancel() and task_restart() (which
should also go for the same reasons). It is used in should also go for the same reasons). It is used in
NxWM::CNxConsole to terminate console tasks and also in NxWM::CNxConsole to terminate console tasks and also in
apps/netutils/thttpd to kill CGI tasks that timeout. apps/netutils/thttpd to kill CGI tasks that timeout. So not
so simple to remove.
Status: Open Status: Open
Priority: Low and not easily removable. Priority: Low and not easily removable.

View File

@ -46,18 +46,6 @@
#if ((defined(CONFIG_BUILD_PROTECTED) && defined(__KERNEL__)) || \ #if ((defined(CONFIG_BUILD_PROTECTED) && defined(__KERNEL__)) || \
defined(CONFIG_BUILD_KERNEL)) && !defined(CONFIG_DISABLE_SIGNALS) defined(CONFIG_BUILD_KERNEL)) && !defined(CONFIG_DISABLE_SIGNALS)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
/****************************************************************************
* Private Data
****************************************************************************/
/****************************************************************************
* Private Functions
****************************************************************************/
/**************************************************************************** /****************************************************************************
* Public Functions * Public Functions
****************************************************************************/ ****************************************************************************/

View File

@ -323,10 +323,9 @@ struct child_status_s
#endif #endif
/* struct pthread_cleanup_s ******************************************************/ /* struct pthread_cleanup_s ******************************************************/
#ifdef CONFIG_PTHREAD_CLEANUP
/* This structure describes one element of the pthread cleanup stack */ /* This structure describes one element of the pthread cleanup stack */
#ifdef CONFIG_PTHREAD_CLEANUP
struct pthread_cleanup_s struct pthread_cleanup_s
{ {
pthread_cleanup_t pc_cleaner; /* Cleanup callback address */ pthread_cleanup_t pc_cleaner; /* Cleanup callback address */

View File

@ -116,9 +116,16 @@ int pthread_cancel(pthread_t thread)
} }
#ifdef CONFIG_PTHREAD_CLEANUP #ifdef CONFIG_PTHREAD_CLEANUP
/* Perform any stack pthread clean-up callbacks */ /* Perform any stack pthread clean-up callbacks.
*
* REVISIT: In this case, the clean-up callback will execute on the
* thread of the caller of pthread cancel, not on the thread of
* the thread-to-be-canceled. Is that an issue? Presumably they
* are both within the same group and within the same process address
* space.
*/
pthread_cleanup_popall(tcb); pthread_cleanup_popall(tcb);
#endif #endif
/* Complete pending join operations */ /* Complete pending join operations */

View File

@ -77,6 +77,10 @@ static inline void task_atexit(FAR struct tcb_s *tcb)
* REVISIT: This is a security problem In the PROTECTED and KERNEL builds: * REVISIT: This is a security problem In the PROTECTED and KERNEL builds:
* We must not call the registered function in supervisor mode! See also * We must not call the registered function in supervisor mode! See also
* on_exit() and pthread_cleanup_pop() callbacks. * on_exit() and pthread_cleanup_pop() callbacks.
*
* REVISIT: In the case of task_delete(), the callback would execute in
* the context the caller of task_delete() cancel, not in the context of
* the exiting task (or process).
*/ */
if (group && group->tg_nmembers == 1) if (group && group->tg_nmembers == 1)
@ -141,6 +145,10 @@ static inline void task_onexit(FAR struct tcb_s *tcb, int status)
* REVISIT: This is a security problem In the PROTECTED and KERNEL builds: * REVISIT: This is a security problem In the PROTECTED and KERNEL builds:
* We must not call the registered function in supervisor mode! See also * We must not call the registered function in supervisor mode! See also
* atexit() and pthread_cleanup_pop() callbacks. * atexit() and pthread_cleanup_pop() callbacks.
*
* REVISIT: In the case of task_delete(), the callback would execute in
* he context the caller of task_delete() cancel, not in the context of
* the exiting task (or process).
*/ */
if (group && group->tg_nmembers == 1) if (group && group->tg_nmembers == 1)