This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx-apps.git
The following commit(s) were added to refs/heads/master by this push:
new 790be6d63 nshlib/nsh_parse: use sh -c replace pthread detach when nsh
background.
790be6d63 is described below
commit 790be6d639f40cc7528e5728aec3f4ba97c1eb3c
Author: buxiasen <[email protected]>
AuthorDate: Fri Oct 18 17:54:18 2024 +0800
nshlib/nsh_parse: use sh -c replace pthread detach when nsh background.
pthread & detach will still quit when parent task exit,
cause nsh_parse clone args leak. nsh should use task instead of thread
this case can reproduce the memory leak.
int main(int argc, FAR char *argv[])
{
printf("Hello, World!!\n");
system("sleep 1 &");
return 0;
}
Signed-off-by: buxiasen <[email protected]>
---
nshlib/nsh_parse.c | 309 +++++++++--------------------------------------------
1 file changed, 52 insertions(+), 257 deletions(-)
diff --git a/nshlib/nsh_parse.c b/nshlib/nsh_parse.c
index 8e92cdcfc..dd1a9aa97 100644
--- a/nshlib/nsh_parse.c
+++ b/nshlib/nsh_parse.c
@@ -30,7 +30,6 @@
#include <string.h>
#include <errno.h>
#include <debug.h>
-#include <pthread.h>
#include <sched.h>
#include <unistd.h>
@@ -126,19 +125,6 @@
* Private Types
****************************************************************************/
-/* These structure describes the parsed command line */
-
-#ifndef CONFIG_NSH_DISABLEBG
-struct cmdarg_s
-{
- FAR struct nsh_vtbl_s *vtbl; /* For front-end interaction */
- int fd_in; /* FD for output redirection */
- int fd_out; /* FD for output redirection */
- int argc; /* Number of arguments in argv */
- FAR char *argv[MAX_ARGV_ENTRIES]; /* Argument list */
-};
-#endif
-
/* This structure describes the allocation list */
#ifdef HAVE_MEMLIST
@@ -174,13 +160,6 @@ static void nsh_alist_free(FAR struct nsh_vtbl_s *vtbl,
FAR struct nsh_alist_s *alist);
#endif
-#ifndef CONFIG_NSH_DISABLEBG
-static void nsh_releaseargs(struct cmdarg_s *arg);
-static pthread_addr_t nsh_child(pthread_addr_t arg);
-static struct cmdarg_s *nsh_cloneargs(FAR struct nsh_vtbl_s *vtbl,
- int fd_in, int fd_out, int argc, FAR char *argv[]);
-#endif
-
static int nsh_saveresult(FAR struct nsh_vtbl_s *vtbl, bool result);
static int nsh_execute(FAR struct nsh_vtbl_s *vtbl,
int argc, FAR char *argv[], FAR const char *redirfile_in,
@@ -443,107 +422,6 @@ static void nsh_alist_free(FAR struct nsh_vtbl_s *vtbl,
}
#endif
-/****************************************************************************
- * Name: nsh_releaseargs
- ****************************************************************************/
-
-#ifndef CONFIG_NSH_DISABLEBG
-static void nsh_releaseargs(struct cmdarg_s *arg)
-{
- FAR struct nsh_vtbl_s *vtbl = arg->vtbl;
- int i;
-
- /* If the output was redirected, then file descriptor should
- * be closed. The created task has its one, independent copy of
- * the file descriptor
- */
-
- if (vtbl->np.np_redir_out)
- {
- close(arg->fd_out);
- }
-
- /* Same for the input */
-
- if (vtbl->np.np_redir_in)
- {
- close(arg->fd_in);
- }
-
- /* Released the cloned vtbl instance */
-
- nsh_release(vtbl);
-
- /* Release the cloned args */
-
- for (i = 0; i < arg->argc; i++)
- {
- free(arg->argv[i]);
- }
-
- free(arg);
-}
-#endif
-
-/****************************************************************************
- * Name: nsh_child
- ****************************************************************************/
-
-#ifndef CONFIG_NSH_DISABLEBG
-static pthread_addr_t nsh_child(pthread_addr_t arg)
-{
- struct cmdarg_s *carg = (struct cmdarg_s *)arg;
- int ret;
-
- _info("BG %s\n", carg->argv[0]);
-
- /* Execute the specified command on the child thread */
-
- ret = nsh_command(carg->vtbl, carg->argc, carg->argv);
-
- /* Released the cloned arguments */
-
- _info("BG %s complete\n", carg->argv[0]);
- nsh_releaseargs(carg);
-
- /* Detach from the pthread since we are not going to join with it.
- * Otherwise, we would have a memory leak.
- */
-
- pthread_detach(pthread_self());
- return (pthread_addr_t)((uintptr_t)ret);
-}
-#endif
-
-/****************************************************************************
- * Name: nsh_cloneargs
- ****************************************************************************/
-
-#ifndef CONFIG_NSH_DISABLEBG
-static struct cmdarg_s *nsh_cloneargs(FAR struct nsh_vtbl_s *vtbl,
- int fd_in, int fd_out, int argc,
- FAR char *argv[])
-{
- struct cmdarg_s *ret = (struct cmdarg_s *)zalloc(sizeof(struct cmdarg_s));
- int i;
-
- if (ret)
- {
- ret->vtbl = vtbl;
- ret->fd_in = fd_in;
- ret->fd_out = fd_out;
- ret->argc = argc;
-
- for (i = 0; i < argc; i++)
- {
- ret->argv[i] = strdup(argv[i]);
- }
- }
-
- return ret;
-}
-#endif
-
/****************************************************************************
* Name: nsh_saveresult
****************************************************************************/
@@ -616,8 +494,6 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl,
FAR const char *redirfile_in,
FAR const char *redirfile_out, int oflags)
{
- int fd_in = STDIN_FILENO;
- int fd_out = STDOUT_FILENO;
int ret;
/* DO NOT CHANGE THE ORDERING OF THE FOLLOWING STEPS
@@ -713,42 +589,6 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl,
#endif
- /* Redirected output? */
-
- if (vtbl->np.np_redir_out)
- {
- /* Open the redirection file. This file will eventually
- * be closed by a call to either nsh_release (if the command
- * is executed in the background) or by nsh_undirect if the
- * command is executed in the foreground.
- */
-
- fd_out = open(redirfile_out, oflags, 0666);
- if (fd_out < 0)
- {
- nsh_error(vtbl, g_fmtcmdfailed, argv[0], "open", NSH_ERRNO);
- goto errout;
- }
- }
-
- /* Redirected input? */
-
- if (vtbl->np.np_redir_in)
- {
- /* Open the redirection file. This file will eventually
- * be closed by a call to either nsh_release (if the command
- * is executed in the background) or by nsh_undirect if the
- * command is executed in the foreground.
- */
-
- fd_in = open(redirfile_in, O_RDONLY, 0);
- if (fd_in < 0)
- {
- nsh_error(vtbl, g_fmtcmdfailed, argv[0], "open", NSH_ERRNO);
- goto errout;
- }
- }
-
/* Handle the case where the command is executed in background.
* However is app is to be started as built-in new process will
* be created anyway, so skip this step.
@@ -757,108 +597,79 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl,
#ifndef CONFIG_NSH_DISABLEBG
if (vtbl->np.np_bg)
{
- struct sched_param param;
- struct nsh_vtbl_s *bkgvtbl;
- struct cmdarg_s *args;
- pthread_attr_t attr;
- pthread_t thread;
-
- /* Get a cloned copy of the vtbl with reference count=1.
- * after the command has been processed, the nsh_release() call
- * at the end of nsh_child() will destroy the clone.
- */
+ FAR char *sh_argv[4];
+ FAR char *sh_cmd = "sh";
+ int i;
- bkgvtbl = nsh_clone(vtbl);
- if (!bkgvtbl)
- {
- goto errout_with_redirect;
- }
+ DEBUGASSERT(strncmp(argv[0], sh_cmd, 3) != 0);
- /* Create a container for the command arguments */
-
- args = nsh_cloneargs(bkgvtbl, fd_in, fd_out, argc, argv);
- if (!args)
+ sh_argv[0] = sh_cmd;
+ sh_argv[1] = "-c";
+ for (i = 0; i < argc - 1; i++)
{
- nsh_release(bkgvtbl);
- goto errout_with_redirect;
- }
+ FAR char *p_arg = argv[i];
+ size_t len = strlen(p_arg);
- /* Handle redirection of output via a file descriptor */
+ /* Restore from split args to concat args. */
- if (vtbl->np.np_redir_out || vtbl->np.np_redir_in)
- {
- nsh_redirect(bkgvtbl, fd_in, fd_out, NULL);
+ DEBUGASSERT(&p_arg[len + 1] == argv[i + 1]);
+ p_arg[len] = ' ';
}
- /* Get the execution priority of this task */
+ sh_argv[2] = argv[0];
+ sh_argv[3] = NULL;
- ret = sched_getparam(0, ¶m);
- if (ret != 0)
- {
- nsh_error(vtbl, g_fmtcmdfailed, argv[0], "sched_getparm",
- NSH_ERRNO);
+ /* np.np_bg still there, try use nsh_builtin or nsh_fileapp to
+ * dispatch the backgroud by sh -c ""
+ */
- /* NOTE: bkgvtbl is released in nsh_relaseargs() */
+ return nsh_execute(vtbl, 4, sh_argv,
+ redirfile_in, redirfile_out, oflags);
+ }
+ else
+#endif
+ {
+ uint8_t save[SAVE_SIZE];
- nsh_releaseargs(args);
- goto errout;
- }
+ int fd_in = STDIN_FILENO;
+ int fd_out = STDOUT_FILENO;
- /* Determine the priority to execute the command */
+ /* Redirected output? */
- if (vtbl->np.np_nice != 0)
+ if (vtbl->np.np_redir_out)
{
- int priority = param.sched_priority - vtbl->np.np_nice;
- if (vtbl->np.np_nice < 0)
- {
- int max_priority = sched_get_priority_max(SCHED_NSH);
- if (priority > max_priority)
- {
- priority = max_priority;
- }
- }
- else
+ /* Open the redirection file. This file will eventually
+ * be closed by a call to either nsh_release (if the command
+ * is executed in the background) or by nsh_undirect if the
+ * command is executed in the foreground.
+ */
+
+ fd_out = open(redirfile_out, oflags, 0666);
+ if (fd_out < 0)
{
- int min_priority = sched_get_priority_min(SCHED_NSH);
- if (priority < min_priority)
- {
- priority = min_priority;
- }
+ nsh_error(vtbl, g_fmtcmdfailed, argv[0], "open", NSH_ERRNO);
+ return nsh_saveresult(vtbl, true);
}
-
- param.sched_priority = priority;
}
- /* Set up the thread attributes */
+ /* Redirected input? */
- pthread_attr_init(&attr);
- pthread_attr_setschedpolicy(&attr, SCHED_NSH);
- pthread_attr_setschedparam(&attr, ¶m);
-
- /* Execute the command as a separate thread at the appropriate
- * priority.
- */
-
- ret = pthread_create(&thread, &attr, nsh_child, (pthread_addr_t)args);
- if (ret != 0)
+ if (vtbl->np.np_redir_in)
{
- nsh_error(vtbl, g_fmtcmdfailed, argv[0], "pthread_create",
- NSH_ERRNO_OF(ret));
-
- /* NOTE: bkgvtbl is released in nsh_relaseargs() */
+ /* Open the redirection file. This file will eventually
+ * be closed by a call to either nsh_release (if the command
+ * is executed in the background) or by nsh_undirect if the
+ * command is executed in the foreground.
+ */
- nsh_releaseargs(args);
- goto errout;
+ fd_in = open(redirfile_in, O_RDONLY, 0);
+ if (fd_in < 0)
+ {
+ nsh_error(vtbl, g_fmtcmdfailed, argv[0], "open", NSH_ERRNO);
+ return nsh_saveresult(vtbl, true);
+ }
}
- nsh_output(vtbl, "%s [%d:%d]\n", argv[0], thread,
- param.sched_priority);
- }
- else
-#endif
- {
- uint8_t save[SAVE_SIZE];
-
/* Handle redirection of stdin/stdout file descriptor */
if (vtbl->np.np_redir_out || vtbl->np.np_redir_in)
@@ -890,7 +701,7 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl,
if (ret < 0)
{
- goto errout;
+ return nsh_saveresult(vtbl, true);
}
}
@@ -899,22 +710,6 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl,
*/
return nsh_saveresult(vtbl, false);
-
-#ifndef CONFIG_NSH_DISABLEBG
-errout_with_redirect:
- if (vtbl->np.np_redir_out)
- {
- close(fd_out);
- }
-
- if (vtbl->np.np_redir_in)
- {
- close(fd_in);
- }
-#endif
-
-errout:
- return nsh_saveresult(vtbl, true);
}
/****************************************************************************
