From 6ab00f4a9382ac813b652f0d449d2c3e2c55f42a Mon Sep 17 00:00:00 2001 From: patacongo Date: Fri, 18 Jan 2013 01:52:42 +0000 Subject: [PATCH] Add internal API task_reparent(), used in posix_spawn(). Move libc/spawn/lib_ps.c to sched/task_posixspawn.c; Move libc/spawn/spawn.h to include/nuttx/spawn.h git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@5531 42af7a65-404d-4744-a932-0658087f49c3 --- builtin/exec_builtin.c | 44 +++++++++++++++++++++++++++++++----------- nshlib/nsh_builtin.c | 26 ++++++++++++++++--------- nshlib/nsh_fileapps.c | 21 +++++++++++--------- 3 files changed, 62 insertions(+), 29 deletions(-) diff --git a/builtin/exec_builtin.c b/builtin/exec_builtin.c index 803d1ef34..60e8b742d 100644 --- a/builtin/exec_builtin.c +++ b/builtin/exec_builtin.c @@ -124,7 +124,7 @@ static void bultin_semtake(FAR sem_t *sem) do { ret = sem_wait(sem); - ASSERT(ret == 0 || errno == EINTR); + ASSERT(ret == 0 || get_errno() == EINTR); } while (ret != 0); } @@ -152,7 +152,7 @@ static int builtin_taskcreate(int index, FAR const char **argv) if (b == NULL) { - errno = ENOENT; + set_errno(ENOENT); return ERROR; } @@ -228,7 +228,7 @@ static int builtin_proxy(int argc, char *argv[]) { /* Remember the errno value. ret is already set to ERROR */ - g_builtin_parms.errcode = errno; + g_builtin_parms.errcode = get_errno(); sdbg("ERROR: open of %s failed: %d\n", g_builtin_parms.redirfile, g_builtin_parms.errcode); } @@ -246,7 +246,7 @@ static int builtin_proxy(int argc, char *argv[]) ret = dup2(fd, 1); if (ret < 0) { - g_builtin_parms.errcode = errno; + g_builtin_parms.errcode = get_errno(); sdbg("ERROR: dup2 failed: %d\n", g_builtin_parms.errcode); } @@ -266,12 +266,18 @@ static int builtin_proxy(int argc, char *argv[]) ret = builtin_taskcreate(g_builtin_parms.index, g_builtin_parms.argv); if (ret < 0) { - g_builtin_parms.errcode = errno; + g_builtin_parms.errcode = get_errno(); sdbg("ERROR: builtin_taskcreate failed: %d\n", g_builtin_parms.errcode); } } + /* NOTE: There is a logical error here if CONFIG_SCHED_HAVE_PARENT is + * defined: The new task is the child of this proxy task, not the + * original caller. As a consequence, operations like waitpid() will + * fail on the caller's thread. + */ + /* Post the semaphore to inform the parent task that we have completed * what we need to do. */ @@ -340,11 +346,21 @@ static inline int builtin_startproxy(int index, FAR const char **argv, ret = sched_getparam(0, ¶m); if (ret < 0) { - errcode = errno; + errcode = get_errno(); sdbg("ERROR: sched_getparam failed: %d\n", errcode); - goto errout; + goto errout_with_sem; } + /* Disable pre-emption so that the proxy does not run until we waitpid + * is called. This is probably unnecessary since the builtin_proxy has + * the same priority as this thread; it should be schedule behind this + * task in the ready-to-run list. + */ + +#ifdef CONFIG_SCHED_WAITPID + sched_lock(); +#endif + /* Start the intermediary/proxy task at the same priority as the parent task. */ proxy = TASK_CREATE("builtin_proxy", param.sched_priority, @@ -352,9 +368,9 @@ static inline int builtin_startproxy(int index, FAR const char **argv, (FAR const char **)NULL); if (proxy < 0) { - errcode = errno; + errcode = get_errno(); sdbg("ERROR: Failed to start builtin_proxy: %d\n", errcode); - goto errout; + goto errout_with_lock; } /* Wait for the proxy to complete its job. We could use waitpid() @@ -365,7 +381,8 @@ static inline int builtin_startproxy(int index, FAR const char **argv, ret = waitpid(proxy, &status, 0); if (ret < 0) { - sdbg("ERROR: waitpid() failed: %d\n", errno); + sdbg("ERROR: waitpid() failed: %d\n", get_errno()); + goto errout_with_lock; } #else bultin_semtake(&g_builtin_execsem); @@ -377,7 +394,12 @@ static inline int builtin_startproxy(int index, FAR const char **argv, builtin_semgive(&g_builtin_parmsem); return g_builtin_parms.result; -errout: +errout_with_lock: +#ifdef CONFIG_SCHED_WAITPID + sched_unlock(); +#endif + +errout_with_sem: set_errno(errcode); builtin_semgive(&g_builtin_parmsem); return ERROR; diff --git a/nshlib/nsh_builtin.c b/nshlib/nsh_builtin.c index 343a0824b..0d5a0231c 100644 --- a/nshlib/nsh_builtin.c +++ b/nshlib/nsh_builtin.c @@ -131,12 +131,14 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, { /* The application was successfully started with pre-emption disabled. * In the simplest cases, the application will not have run because the - * the scheduler is locked. but in the case were I/O redirected, a - * proxy task ran and, as result, so may have the application. + * the scheduler is locked. But in the case where I/O was redirected, a + * proxy task ran and broke our lock. As result, the application may + * have aso ran if its priority was higher than than the priority of + * this thread. * - * If the application did not run and if the application was not - * backgrounded, then we need to wait here for the application to - * exit. This only works works with the following options: + * If the application did not run to completion and if the application + * was not backgrounded, then we need to wait here for the application + * to exit. This only works works with the following options: * * - CONFIG_NSH_DISABLEBG - Do not run commands in background * - CONFIG_SCHED_WAITPID - Required to run external commands in @@ -156,11 +158,17 @@ int nsh_builtin(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, { int rc = 0; - /* Wait for the application to exit. We did locked the scheduler + /* Wait for the application to exit. We did lock the scheduler * above, but that does not guarantee that the application did not - * run in the case where I/O was redirected. The scheduler will - * be unlocked while waitpid is waiting and if the application has - * not yet run, it will be able to to do so. + * already run to completion in the case where I/O was redirected. + * Here the scheduler will be unlocked while waitpid is waiting + * and if the application has not yet run, it will now be able to + * do so. + * + * Also, if CONFIG_SCHED_HAVE_PARENT is defined waitpid() might fail + * even if task is still active: If the I/O was re-directed by a + * proxy task, then the ask is a child of the proxy, and not this + * task. waitpid() fails with ECHILD in either case. */ ret = waitpid(ret, &rc, 0); diff --git a/nshlib/nsh_fileapps.c b/nshlib/nsh_fileapps.c index 92146a68d..9ff230f1a 100644 --- a/nshlib/nsh_fileapps.c +++ b/nshlib/nsh_fileapps.c @@ -168,12 +168,14 @@ int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, { /* The application was successfully started with pre-emption disabled. * In the simplest cases, the application will not have run because the - * the scheduler is locked. but in the case were I/O redirected, a - * proxy task ran and, as result, so may have the application. + * the scheduler is locked. But in the case where I/O was redirected, a + * proxy task ran and broke our lock. As result, the application may + * have aso ran if its priority was higher than than the priority of + * this thread. * - * If the application did not run and if the application was not - * backgrounded, then we need to wait here for the application to - * exit. This only works works with the following options: + * If the application did not run to completion and if the application + * was not backgrounded, then we need to wait here for the application + * to exit. This only works works with the following options: * * - CONFIG_NSH_DISABLEBG - Do not run commands in background * - CONFIG_SCHED_WAITPID - Required to run external commands in @@ -193,11 +195,12 @@ int nsh_fileapp(FAR struct nsh_vtbl_s *vtbl, FAR const char *cmd, { int rc = 0; - /* Wait for the application to exit. We did locked the scheduler + /* Wait for the application to exit. We did lock the scheduler * above, but that does not guarantee that the application did not - * run in the case where I/O was redirected. The scheduler will - * be unlocked while waitpid is waiting and if the application has - * not yet run, it will be able to to do so. + * already run to completion in the case where I/O was redirected. + * Here the scheduler will be unlocked while waitpid is waiting + * and if the application has not yet run, it will now be able to + * do so. */ ret = waitpid(pid, &rc, 0);