diff --git a/src/cmd/ksh93/bltins/typeset.c b/src/cmd/ksh93/bltins/typeset.c index 38b88d22e..4ddeb2789 100644 --- a/src/cmd/ksh93/bltins/typeset.c +++ b/src/cmd/ksh93/bltins/typeset.c @@ -1277,12 +1277,6 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp) while(r = optget(argv,name)) switch(r) { case 'f': - /* - * Unsetting functions in a virtual/non-forked subshell doesn't work due to limitations - * in the subshell function tree mechanism. Work around this by forking the subshell. - */ - if(shp->subshell && !shp->subshare) - sh_subfork(); troot = sh_subfuntree(1); break; case 'a': @@ -1388,33 +1382,24 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp) _nv_unset(np,0); if(troot==shp->var_tree && shp->st.real_fun && (dp=shp->var_tree->walk) && dp==shp->st.real_fun->sdict) nv_delete(np,dp,NV_NOFREE); - else if(isfun && !(np->nvalue.rp && np->nvalue.rp->running)) + else if(isfun) { - nv_delete(np,troot,0); - /* - * If we have just unset a function in a subshell tree that overrode a function by the same - * name in the main shell, then the above nv_delete() call incorrectly restores the function - * from the main shell scope. So walk though troot's parent views and delete any such zombie - * functions. Note that this only works because 'unset -f' now forks if we're in a subshell. - */ - Dt_t *troottmp = troot; - while((troottmp = troottmp->view) && (np = nv_search(name,troottmp,0)) && is_afunction(np)) - nv_delete(np,troottmp,0); + if(troot!=shp->fun_base) + nv_offattr(np,NV_FUNCTION); /* invalidate */ + else if(!(np->nvalue.rp && np->nvalue.rp->running)) + nv_delete(np,troot,0); } /* The alias has been unset by call to _nv_unset, remove it from the tree */ else if(troot==shp->alias_tree) nv_delete(np,troot,nofree_attr); -#if 0 - /* causes unsetting local variable to expose global */ - else if(shp->var_tree==troot && shp->var_tree!=shp->var_base && nv_search((char*)np,shp->var_tree,HASH_BUCKET|HASH_NOSCOPE)) - nv_delete(np,shp->var_tree,0); -#endif else nv_close(np); } else if(troot==shp->alias_tree) r = 1; + else if(troot==shp->fun_tree && troot!=shp->fun_base && nv_search(name,shp->fun_base,0)) + nv_open(name,troot,NV_NOSCOPE); /* create dummy virtual subshell node without NV_FUNCTION attribute */ } sh_popcontext(shp,&buff); return(r); diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h index 6ebc299cd..e9ec3501c 100644 --- a/src/cmd/ksh93/include/defs.h +++ b/src/cmd/ksh93/include/defs.h @@ -165,6 +165,7 @@ struct shared int path_err; /* last error on path search */ \ Dt_t *track_tree; /* for tracked aliases */ \ Dt_t *var_base; /* global level variables */ \ + Dt_t *fun_base; /* global level functions */ \ Dt_t *openmatch; \ Namval_t *namespace; /* current active namespace */ \ Namval_t *last_table; /* last table used in last nv_open */ \ diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index 64dc6b78c..6a9647330 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -1821,7 +1821,7 @@ static Init_t *nv_init(Shell_t *shp) dtuserdata(shp->track_tree,shp,1); shp->bltin_tree = sh_inittree(shp,(const struct shtable2*)shtab_builtins); dtuserdata(shp->bltin_tree,shp,1); - shp->fun_tree = dtopen(&_Nvdisc,Dtoset); + shp->fun_base = shp->fun_tree = dtopen(&_Nvdisc,Dtoset); dtuserdata(shp->fun_tree,shp,1); dtview(shp->fun_tree,shp->bltin_tree); nv_mount(DOTSHNOD, "type", shp->typedict=dtopen(&_Nvdisc,Dtoset)); diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index a245523aa..00aaf2185 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -426,12 +426,21 @@ static void nv_restore(struct subshell *sp) Dt_t *sh_subtracktree(int create) { register struct subshell *sp = subshell_data; - if(create && sh.subshell && !sh.subshare && sp && !sp->strack) + if(create && sh.subshell) { - sp->strack = dtopen(&_Nvdisc,Dtset); - dtuserdata(sp->strack,&sh,1); - dtview(sp->strack,sh.track_tree); - sh.track_tree = sp->strack; + if(sh.subshare) + { + while(sp->subshare) /* move up as long as parent is also a ${ subshare; } */ + sp = sp->prev; + sp = sp->prev; /* move to first non-subshare parent */ + } + if(sp && !sp->strack) + { + sp->strack = dtopen(&_Nvdisc,Dtset); + dtuserdata(sp->strack,&sh,1); + dtview(sp->strack,sh.track_tree); + sh.track_tree = sp->strack; + } } return(sh.track_tree); } @@ -443,41 +452,25 @@ Dt_t *sh_subtracktree(int create) Dt_t *sh_subfuntree(int create) { register struct subshell *sp = subshell_data; - if(create && sh.subshell && !sh.subshare && sp && !sp->sfun) + if(create && sh.subshell) { - sp->sfun = dtopen(&_Nvdisc,Dtoset); - dtuserdata(sp->sfun,&sh,1); - dtview(sp->sfun,sh.fun_tree); - sh.fun_tree = sp->sfun; + if(sh.subshare) + { + while(sp->subshare) /* move up as long as parent is also a ${ subshare; } */ + sp = sp->prev; + sp = sp->prev; /* move to first non-subshare parent */ + } + if(sp && !sp->sfun) + { + sp->sfun = dtopen(&_Nvdisc,Dtoset); + dtuserdata(sp->sfun,&sh,1); + dtview(sp->sfun,sh.fun_tree); + sh.fun_tree = sp->sfun; + } } return(sh.fun_tree); } -/* - * Remove and free a subshell table at *root after leaving a virtual subshell. - * Pass 'fun' as nonzero when removing a subshell's shell functions. - */ -static void table_unset(Shell_t *shp,register Dt_t *root,int fun) -{ - register Namval_t *np,*nq; - int flag; - for(np=(Namval_t*)dtfirst(root);np;np=nq) - { - nq = (Namval_t*)dtnext(root,np); - flag=0; - /* Check for autoloaded function; it must not be freed. */ - if(fun && np->nvalue.rp && np->nvalue.rp->fname && shp->fpathdict - && nv_search(np->nvalue.rp->fname,shp->fpathdict,0)) - { - np->nvalue.rp->fdict = 0; - flag = NV_NOFREE; - } - else - _nv_unset(np,NV_RDONLY); - nv_delete(np,root,flag|NV_FUNCTION); - } -} - int sh_subsavefd(register int fd) { register struct subshell *sp = subshell_data; @@ -808,30 +801,47 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) /* Clean up subshell hash table. */ if(sp->strack) { - Namval_t *np, *prev_np; + Namval_t *np, *next_np; /* Detach this scope from the unified view. */ shp->track_tree = dtview(sp->strack,0); - /* Delete (free) all elements of the subshell hash table. To allow dtnext() to - set the pointer to the next item, we have to delete one item behind the loop. */ - prev_np = 0; - np = (Namval_t*)dtfirst(sp->strack); - while(np) + /* Free all elements of the subshell hash table. */ + for(np = (Namval_t*)dtfirst(sp->strack); np; np = next_np) { - if(prev_np) - nv_delete(prev_np,sp->strack,0); - prev_np = np; - np = (Namval_t*)dtnext(sp->strack,np); + next_np = (Namval_t*)dtnext(sp->strack,np); + nv_delete(np,sp->strack,0); } - if(prev_np) - nv_delete(prev_np,sp->strack,0); /* Close and free the table itself. */ dtclose(sp->strack); } /* Clean up subshell function table. */ if(sp->sfun) { + Namval_t *np, *next_np; + /* Detach this scope from the unified view. */ shp->fun_tree = dtview(sp->sfun,0); - table_unset(shp,sp->sfun,1); + /* Free all elements of the subshell function table. */ + for(np = (Namval_t*)dtfirst(sp->sfun); np; np = next_np) + { + next_np = (Namval_t*)dtnext(sp->sfun,np); + if(!np->nvalue.rp) + { + /* Dummy node created by unall() to mask parent shell function. */ + nv_delete(np,sp->sfun,0); + continue; + } + nv_onattr(np,NV_FUNCTION); /* in case invalidated by unall() */ + if(np->nvalue.rp->fname && shp->fpathdict && nv_search(np->nvalue.rp->fname,shp->fpathdict,0)) + { + /* Autoloaded function. It must not be freed. */ + np->nvalue.rp->fdict = 0; + nv_delete(np,sp->sfun,NV_FUNCTION|NV_NOFREE); + } + else + { + _nv_unset(np,NV_RDONLY); + nv_delete(np,sp->sfun,NV_FUNCTION); + } + } dtclose(sp->sfun); } n = shp->st.trapmax-savst.trapmax; diff --git a/src/cmd/ksh93/tests/leaks.sh b/src/cmd/ksh93/tests/leaks.sh index bbb7eec5b..58337f171 100755 --- a/src/cmd/ksh93/tests/leaks.sh +++ b/src/cmd/ksh93/tests/leaks.sh @@ -195,6 +195,40 @@ after=$(getmem) err_exit_if_leak 'POSIX function defined in virtual subshell' typeset -f foo >/dev/null && err_exit 'POSIX function leaks out of subshell' +# Unsetting a function in a virtual subshell + +function foo { echo bar; } +before=$(getmem) +for ((i=0; i < N; i++)) +do (unset -f foo) +done +after=$(getmem) +err_exit_if_leak 'ksh function unset in virtual subshell' +typeset -f foo >/dev/null || err_exit 'ksh function unset in subshell was unset in main shell' + +foo() { echo bar; } +before=$(getmem) +for ((i=0; i < N; i++)) +do (unset -f foo) +done +after=$(getmem) +err_exit_if_leak 'POSIX function unset in virtual subshell' +typeset -f foo >/dev/null || err_exit 'POSIX function unset in subshell was unset in main shell' + +before=$(getmem) +for ((i=0; i < N; i++)) +do (function foo { echo baz; }; unset -f foo) +done +after=$(getmem) +err_exit_if_leak 'ksh function defined and unset in virtual subshell' + +before=$(getmem) +for ((i=0; i < N; i++)) +do (foo() { echo baz; }; unset -f foo) +done +after=$(getmem) +err_exit_if_leak 'POSIX function defined and unset in virtual subshell' + # ====== # Sourcing a dot script in a virtual subshell diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh index 968a29f62..bde31f1e6 100755 --- a/src/cmd/ksh93/tests/subshell.sh +++ b/src/cmd/ksh93/tests/subshell.sh @@ -656,6 +656,7 @@ v=$("$SHELL" -c '. "$1"' x "$a") && [[ $v == ok ]] || err_exit "fail: more fun 4 # ...multiple levels of subshell func() { echo mainfunction; } v=$( + set +x ( func() { echo sub1; } ( @@ -664,20 +665,34 @@ v=$( func() { echo sub3; } func PATH=/dev/null - unset -f func + dummy=${ dummy=${ dummy=${ dummy=${ unset -f func; }; }; }; }; # test subshare within subshell func 2>/dev/null (($? == 127)) && echo ok_nonexistent || echo fail_zombie ) func ) func - ) - func + ) 2>&1 + func 2>&1 ) expect=$'sub3\nok_nonexistent\nsub2\nsub1\nmainfunction' [[ $v == "$expect" ]] \ || err_exit "multi-level subshell function failure (expected $(printf %q "$expect"), got $(printf %q "$v"))" +# ... https://github.com/ksh93/ksh/issues/228 +fail() { + echo 'Failure' +} +exp=Success +got=$( + foo() { true; } # Define function before forking + ulimit -t unlimited 2>/dev/null # Fork the subshell + unset -f fail + PATH=/dev/null fail 2>/dev/null || echo "$exp" +) +[[ $got == "$exp" ]] || err_exit 'unset -f fails in forked subshells if a function is defined before forking' \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" + # ====== # Unsetting or redefining aliases within subshells @@ -945,6 +960,29 @@ v=main (v=sub; (d=${ v=shared; }; [[ $v == shared ]]) ) || err_exit "shared comsub in nested subshell wrongly scoped (2)" [[ $v == main ]] || err_exit "shared comsub leaks out of subshell (7)" +# ...multiple levels of subshell +v=main +got=$( + ( + v=sub1 + ( + v=sub2 + ( + v=sub3 + echo $v + dummy=${ dummy=${ dummy=${ dummy=${ unset v; }; }; }; }; # test subshare within subshell + [[ -n $v || -v v ]] && echo fail_zombie || echo ok_nonexistent + ) + echo $v + ) + echo $v + ) + echo $v +) +exp=$'sub3\nok_nonexistent\nsub2\nsub1\nmain' +[[ $got == "$exp" ]] || err_exit "multi-level subshell function failure" \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" + # ====== # After the memory leak patch for rhbz#982142, this minor regression was introduced: # if a backtick command substitution expanded an alias, an extra space was inserted.