Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fundamental flaw in forking checks for shared-state command substitution #289

Closed
McDutchie opened this issue Apr 26, 2021 · 1 comment
Closed
Labels
bug Something is not working

Comments

@McDutchie
Copy link

McDutchie commented Apr 26, 2021

The code contains various checks to see if a subshell needs to fork, like this one in the ulimit builtin:

if(shp->subshell && !shp->subshare)
sh_subfork();

All checks of this form are fatally broken, as each one of them causes shared-state command substitutions to ignore parent virtual subshells.

Taking ulimit as an example again:

$ ulimit -t
unlimited
$ (ulimit -t 1); ulimit -t         
unlimited
$ (dummy=${ ulimit -t 1; }); ulimit -t
1

The last command wrongly imposed a 1-second CPU time limit on the main shell.

So all these checks need updating. They need a fix similar to the one applied to sh_assignok() in 911d6b0. That fix should be applied to sh_subfork(), but it will be complicated.

The places to look are:

$ grep -rwn sh_subfork src/cmd/ksh93/                                                                                              
src/cmd/ksh93//include/shell.h:229:extern void		sh_subfork(void);
src/cmd/ksh93//bltins/typeset.c:188:		sh_subfork();			/* avoid affecting main shell's alias table */
src/cmd/ksh93//bltins/typeset.c:709:						sh_subfork();
src/cmd/ksh93//bltins/typeset.c:1148:			sh_subfork();
src/cmd/ksh93//bltins/typeset.c:1255:		sh_subfork();
src/cmd/ksh93//bltins/ulimit.c:140:				sh_subfork();
src/cmd/ksh93//bltins/cd_pwd.c:121:			sh_subfork();
src/cmd/ksh93//bltins/misc.c:142:			sh_subfork();
src/cmd/ksh93//sh/io.c:1157:				sh_subfork();
src/cmd/ksh93//sh/xec.c:1603:					sh_subfork();
src/cmd/ksh93//sh/xec.c:2437:					sh_subfork();
src/cmd/ksh93//sh/fault.c:108:		sh_subfork();
src/cmd/ksh93//sh/fault.c:553:				sh_subfork();
src/cmd/ksh93//sh/subshell.c:188:void sh_subfork(void)
src/cmd/ksh93//sh/subshell.c:679:				sh_subfork();			/* ...we have to fork, as we cannot fchdir back to it. */
src/cmd/ksh93//sh/subshell.c:689:					sh_subfork();
@McDutchie McDutchie added the bug Something is not working label Apr 26, 2021
@McDutchie
Copy link
Author

After some experimentation I think it's not currently feasible to fix virtual subshells to coexist correctly with shared-state command substitutions.

An easy fix/workaround to ensure correct behaviour is to fork a virtual subshell before executing a shared-state command substitution within it. That is what I'll do for the time being.

In the long term I think shared-state command substitutions should probably be redesigned to disassociate them completely from the virtual subshell mechanism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working
Projects
None yet
Development

No branches or pull requests

1 participant