variable exported in function in a subshell is also visible in a different subshell

[lijog]

I've observed this issue in a Solaris 11 system with all the ksh93 versions(alpha, beta and the master versions) as well as in Ubuntu 14 ( ksh2012).

Here's a testcase which reproduces the issue.

# cat test1.ksh
# !/usr/bin/ksh

function proxy { 
         export MYVAR="bla" 
        child 
        unset MYVAR 
} 

function child { 
        echo "MYVAR=$MYVAR" >> /var/tmp/debug.log 
} 

function test { 
        $(child) 
        $(proxy) 
        $(child) 
} 

rm /var/tmp/debug.log 
test 
cat /var/tmp/debug.log 
# ./test1.ksh

MYVAR= 
MYVAR=bla 
MYVAR=bla <------------------------------ this should not happen 
# 

The following patch which removes an optimization fixes the issue. If there is any other patch, please let me know.

--- INIT.2012-08-01.old/src/cmd/ksh93/sh/subshell.c     2016-03-01 04:01:06.513890578 -0800
+++ INIT.2012-08-01/src/cmd/ksh93/shsubshell.c  2016-03-01 04:02:43.617872391 -0800
@@ -260,9 +260,6 @@
        shp = sp->shp;
        dp = shp->var_tree;
-       /\* don't bother to save if in newer scope */
-       if(sp->var!=shp->var_tree && sp->var!=shp->var_base && shp->last_root==shp->var_tree)
-               return(np);
      if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np)))
      {
              shp->last_root = ap->table;

The issue seems to be happening because of the following optimization in sh_assignok function in sh/subshell.c.

 /\* don't bother to save if in newer scope */ 
 if(sp->var!=shp->var_tree && shp->last_root==shp->var_tree) 
        return(np); 

This optimization prevents saving the variables in situations where they don't need to be restored. If we remove the optimization, the issue will go away.

[krader1961]

Does anyone know why the proposed fix hasn't been merged? I can reproduce the failure using the provided test case using a ksh93 built from the current beta branch. The proposed fix does make the test case behave correctly. TBD is whether it affects any of the unit tests.

[kdudka]

The patch is nearly impossible to read. Please use markdown to denote code blocks (to avoid interpreting code as markdown and to switch font to monospace).

[dannyweldon]

I just edited the comment to format the code blocks.

[kdudka]

@dannyweldon Thanks! I do not have privileges to do it myself...

@krader1961 The proposed patch makes sense to me. It fixes the problem and does no seem to break anything else...