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]>

Author: jasonbu

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, &param);
-      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, &param);
-
-      /* 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);
 }
 
 /****************************************************************************