From 40b2c3c3d4b97e6e2d05254444fa45a74542d41e Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Wed, 12 Jan 2022 17:55:36 -0800 Subject: [PATCH] alias: Avoid unnecessary forks (re: ec888867) (#417) The code used to fork subshells when creating/changing aliases will always fork, even when the alias tree isn't changed: $ echo $(unalias --man 2> /dev/null; echo $$ ${.sh.pid}) 375097 375107 $ alias foo=bar; echo $(alias -p foo; echo $$ ${.sh.pid}) alias foo=bar 375097 375110 This is a bit inefficient, so this commit avoids forking a subshell unless at least one change is made to the alias table. src/cmd/ksh93/bltins/typeset.c: - b_alias(), b_unalias(): Remove sh_subfork() calls. - setall(): When creating an alias (name contains '='), fork a virtual subshell before calling nv_open() to add the node. - unall(): - When unsetting all aliases (-a), fork subshell before dtclear(). - When unsetting one alias, fork subshell before nv_delete(). - Move sh_pushcontext() and sh_popcontext() expansions so that sh_subfork() is not in between them, as that would cause program flow corruption or a crash. Co-authored-by: Martijn Dekker --- src/cmd/ksh93/bltins/typeset.c | 27 +++++++++++++++++---------- src/cmd/ksh93/tests/alias.sh | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/src/cmd/ksh93/bltins/typeset.c b/src/cmd/ksh93/bltins/typeset.c index 5d88c34c0..f55496718 100644 --- a/src/cmd/ksh93/bltins/typeset.c +++ b/src/cmd/ksh93/bltins/typeset.c @@ -185,6 +185,8 @@ int b_alias(int argc,register char *argv[],Shbltin_t *context) /* 'alias -t', 'hash' */ if(flag&NV_TAGGED) { + if(xflag) + return(0); /* do nothing for 'alias -tx' */ troot = sh_subtracktree(1); /* use hash table */ if(tdata.pflag) tdata.aflag = '+'; /* for 'alias -pt', don't add anything to the hash table */ @@ -193,10 +195,6 @@ int b_alias(int argc,register char *argv[],Shbltin_t *context) if(rflag) /* hash -r: clear hash table */ nv_scan(troot,nv_rehash,(void*)0,NV_TAGGED,NV_TAGGED); } - else if(argv[1] && sh.subshell && !sh.subshare) - sh_subfork(); /* avoid affecting the parent shell's alias table */ - if(xflag && (flag&NV_TAGGED)) - return(0); /* do nothing for 'alias -tx' */ return(setall(argv,flag,troot,&tdata)); } @@ -747,6 +745,8 @@ static int setall(char **argv,register int flag,Dt_t *troot,struct tdata *tp path_alias(np,path_absolute(nv_name(np),NIL(Pathcomp_t*),0)); continue; } + if(troot==sh.alias_tree && sh.subshell && !sh.subshare && strchr(name,'=')) + sh_subfork(); /* avoid affecting the parent shell's alias table */ np = nv_open(name,troot,nvflags|((nvflags&NV_ASSIGN)?0:NV_ARRAY)|((iarray|(nvflags&(NV_REF|NV_NOADD)==NV_REF))?NV_FARRAY:0)); if(!np || (troot==sh.track_tree && nv_isattr(np,NV_NOALIAS))) { @@ -1256,8 +1256,6 @@ int b_set(int argc,register char *argv[],Shbltin_t *context) int b_unalias(int argc,register char *argv[],Shbltin_t *context) { NOT_USED(context); - if(sh.subshell && !sh.subshare) - sh_subfork(); return(unall(argc,argv,sh.alias_tree)); } @@ -1316,12 +1314,17 @@ static int unall(int argc, char **argv, register Dt_t *troot) nflag = NV_NOSCOPE; if(all) { - dtclear(troot); + if(dtfirst(troot)) + { + if(troot==sh.alias_tree && sh.subshell && !sh.subshare) + sh_subfork(); /* avoid affecting the parent shell's alias table */ + dtclear(troot); + } return(r); } - sh_pushcontext(&sh,&buff,1); while(name = *argv++) { + sh_pushcontext(&sh,&buff,1); jmpval = sigsetjmp(buff.buff,0); np = 0; if(jmpval==0) @@ -1333,7 +1336,8 @@ static int unall(int argc, char **argv, register Dt_t *troot) #endif /* SHOPT_NAMESPACE */ np=nv_open(name,troot,NV_NOADD|nflag); } - else + sh_popcontext(&sh,&buff); + if(jmpval) { r = 1; continue; @@ -1392,7 +1396,11 @@ static int unall(int argc, char **argv, register Dt_t *troot) } /* The alias has been unset by call to _nv_unset, remove it from the tree */ else if(troot==sh.alias_tree) + { + if(sh.subshell && !sh.subshare) + sh_subfork(); /* avoid affecting the parent shell's alias table */ nv_delete(np,troot,nofree_attr); + } else nv_close(np); @@ -1402,7 +1410,6 @@ static int unall(int argc, char **argv, register Dt_t *troot) else if(troot==sh.fun_tree && troot!=sh.fun_base && nv_search(name,sh.fun_tree,0)) nv_open(name,troot,NV_NOSCOPE); /* create dummy virtual subshell node without NV_FUNCTION attribute */ } - sh_popcontext(&sh,&buff); return(r); } diff --git a/src/cmd/ksh93/tests/alias.sh b/src/cmd/ksh93/tests/alias.sh index b1eb0bafe..c186a70d6 100755 --- a/src/cmd/ksh93/tests/alias.sh +++ b/src/cmd/ksh93/tests/alias.sh @@ -275,5 +275,23 @@ ret=$? got=$? ((got > 0)) || err_exit "Exit status is zero when alias is passed 256 non-existent aliases" +# ====== +# https://github.com/ksh93/ksh/pull/417 + +alias foo=bar +(unalias -a) +alias foo >/dev/null 2>&1 || err_exit "unalias -a leaked out of subshell" +unalias foo +(alias foo=bar) +alias foo >/dev/null 2>&1 && err_exit "alias leaked out of subshell" + +alias foo=bar +exp="0 $$" +got=$(alias foo='echo "$? $$"'; eval foo) +[[ $got == "$exp" ]] || err_exit "alias in subshell: expected $(printf %q "$exp"), got $(printf %q "$got")" +got=$(unalias foo; echo "$? $$") +[[ $got == "$exp" ]] || err_exit "unalias in subshell: expected $(printf %q "$exp"), got $(printf %q "$got")" +unalias foo + # ====== exit $((Errors<125?Errors:125))