pkg/eval: Move all assignments to the start of the pipeline they belong in.

This avoids race conditions of accessing the local scope. The test case
"x = 1", "put $x | y = (all)" used to contain a race condition but no longer
does.

This addresses #73.
This commit is contained in:
Qi Xiao 2020-03-28 23:49:30 +00:00
parent 9bebf49b40
commit 8c71635ca3
3 changed files with 35 additions and 20 deletions

View File

@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"os"
"strings"
"sync"
"github.com/elves/elvish/pkg/diag"
@ -39,8 +40,16 @@ func (op chunkOp) invoke(fm *Frame) error {
}
func (cp *compiler) pipelineOp(n *parse.Pipeline) effectOp {
saveNewLocals := cp.newLocals
cp.newLocals = nil
formOps := cp.formOps(n.Forms)
newLocals := cp.newLocals
cp.newLocals = saveNewLocals
return makeEffectOp(n,
&pipelineOp{n.Background, n.SourceText(), cp.formOps(n.Forms)})
&pipelineOp{n.Background, n.SourceText(), formOps, newLocals})
}
func (cp *compiler) pipelineOps(ns []*parse.Pipeline) []effectOp {
@ -52,9 +61,10 @@ func (cp *compiler) pipelineOps(ns []*parse.Pipeline) []effectOp {
}
type pipelineOp struct {
bg bool
source string
subops []effectOp
bg bool
source string
subops []effectOp
newLocals []string
}
const pipelineChanBufferSize = 32
@ -64,6 +74,20 @@ func (op *pipelineOp) invoke(fm *Frame) error {
return ErrInterrupted
}
for _, name := range op.newLocals {
var variable vars.Var
if strings.HasSuffix(name, FnSuffix) {
val := Callable(nil)
variable = vars.FromPtr(&val)
} else if strings.HasSuffix(name, NsSuffix) {
val := Ns(nil)
variable = vars.FromPtr(&val)
} else {
variable = vars.FromInit(nil)
}
fm.local[name] = variable
}
if op.bg {
fm = fm.fork("background job" + op.source)
fm.intCh = nil

View File

@ -3,7 +3,6 @@ package eval
import (
"errors"
"fmt"
"strings"
"github.com/elves/elvish/pkg/diag"
"github.com/elves/elvish/pkg/eval/vars"
@ -127,21 +126,10 @@ type varOp struct {
func (op varOp) invoke(fm *Frame) ([]vars.Var, error) {
variable := fm.ResolveVar(op.qname)
if variable == nil {
ns, name := SplitQNameNs(op.qname)
ns, _ := SplitQNameNs(op.qname)
if ns == "" || ns == ":" || ns == "local:" {
// New variable.
// XXX We depend on the fact that this variable will
// immeidately be set.
if strings.HasSuffix(name, FnSuffix) {
val := Callable(nil)
variable = vars.FromPtr(&val)
} else if strings.HasSuffix(name, NsSuffix) {
val := Ns(nil)
variable = vars.FromPtr(&val)
} else {
variable = vars.FromInit(nil)
}
fm.local[name] = variable
// This should have been created as part of pipelineOp; a compiler bug.
return nil, errors.New("compiler bug: new local variable not created in pipeline")
} else {
return nil, fmt.Errorf("new variables can only be created in local scope")
}

View File

@ -17,12 +17,14 @@ type compiler struct {
scopes []staticNs
// Variables captured from outer scopes.
capture staticNs
// New variables created within a pipeline.
newLocals []string
// Information about the source.
srcMeta *Source
}
func compile(b, g staticNs, n *parse.Chunk, src *Source) (op Op, err error) {
cp := &compiler{b, []staticNs{g}, make(staticNs), src}
cp := &compiler{b, []staticNs{g}, make(staticNs), nil, src}
defer func() {
r := recover()
if r == nil {
@ -90,6 +92,7 @@ func (cp *compiler) registerVariableAccess(qname string, set bool) bool {
createLocal := func(name string) bool {
if set && name != "" && !strings.ContainsRune(name[:len(name)-1], ':') {
cp.thisScope().set(name)
cp.newLocals = append(cp.newLocals, name)
return true
}
return false