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 <buxiasen@xiaomi.com>
This commit is contained in:
buxiasen 2024-10-18 17:54:18 +08:00 • committed by Xiang Xiao
parent ec458f0ec7
commit 790be6d639

View file

@ -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;
FAR char *sh_argv[4];
FAR char *sh_cmd = "sh";
int i;
/* 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.
DEBUGASSERT(strncmp(argv[0], sh_cmd, 3) != 0);
sh_argv[0] = sh_cmd;
sh_argv[1] = "-c";
for (i = 0; i < argc - 1; i++)
{
FAR char *p_arg = argv[i];
size_t len = strlen(p_arg);
/* Restore from split args to concat args. */
DEBUGASSERT(&p_arg[len + 1] == argv[i + 1]);
p_arg[len] = ' ';
}
sh_argv[2] = argv[0];
sh_argv[3] = NULL;
/* np.np_bg still there, try use nsh_builtin or nsh_fileapp to
* dispatch the backgroud by sh -c ""
*/
bkgvtbl = nsh_clone(vtbl);
if (!bkgvtbl)
{
goto errout_with_redirect;
}
/* Create a container for the command arguments */
args = nsh_cloneargs(bkgvtbl, fd_in, fd_out, argc, argv);
if (!args)
{
nsh_release(bkgvtbl);
goto errout_with_redirect;
}
/* Handle redirection of output via a file descriptor */
if (vtbl->np.np_redir_out || vtbl->np.np_redir_in)
{
nsh_redirect(bkgvtbl, fd_in, fd_out, NULL);
}
/* Get the execution priority of this task */
ret = sched_getparam(0, &param);
if (ret != 0)
{
nsh_error(vtbl, g_fmtcmdfailed, argv[0], "sched_getparm",
NSH_ERRNO);
/* NOTE: bkgvtbl is released in nsh_relaseargs() */
nsh_releaseargs(args);
goto errout;
}
/* Determine the priority to execute the command */
if (vtbl->np.np_nice != 0)
{
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
{
int min_priority = sched_get_priority_min(SCHED_NSH);
if (priority < min_priority)
{
priority = min_priority;
}
}
param.sched_priority = priority;
}
/* Set up the thread attributes */
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)
{
nsh_error(vtbl, g_fmtcmdfailed, argv[0], "pthread_create",
NSH_ERRNO_OF(ret));
/* NOTE: bkgvtbl is released in nsh_relaseargs() */
nsh_releaseargs(args);
goto errout;
}
nsh_output(vtbl, "%s [%d:%d]\n", argv[0], thread,
param.sched_priority);
return nsh_execute(vtbl, 4, sh_argv,
redirfile_in, redirfile_out, oflags);
}
else
#endif
{
uint8_t save[SAVE_SIZE];
int fd_in = STDIN_FILENO;
int fd_out = STDOUT_FILENO;
/* 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);
return nsh_saveresult(vtbl, true);
}
}
/* 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);
return nsh_saveresult(vtbl, true);
}
}
/* 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);
}
/****************************************************************************