Lock EnvPathList correctly. This fixes #146.

Although there are no explicit writes to $paths in the test cases, there is
one hidden write: when the first search on external commands is done. Since
the internal cache of EnvPathList starts empty, EnvPathList.get() notices that
it needs to fetch the environment to update itself. However, the update first
modifies .cachedValue and then .cachedPaths. If another goroutine uses .get()
in the middle, it would think that the cache is actually up to date and uses
the outdated .cachedPaths, which is empty.

EnvPathList.get() is now correctly guarded by a write lock. The offending test
case has been uncommented as well.
This commit is contained in:
Qi Xiao 2016-02-20 22:25:26 +01:00
parent bb1e5954f2
commit f001c783d5
2 changed files with 4 additions and 6 deletions

View File

@ -108,8 +108,8 @@ func (epl *EnvPathList) IndexSet(idx, v Value) {
}
func (epl *EnvPathList) get() []string {
epl.RLock()
defer epl.RUnlock()
epl.Lock()
defer epl.Unlock()
value := os.Getenv(epl.envName)
if value == epl.cachedValue {

View File

@ -44,10 +44,8 @@ var evalTests = []struct {
// Pipelines.
// Pure byte pipeline
/*
{`echo "Albert\nAllan\nAlbraham\nBerlin" | sed s/l/1/g | grep e`,
[]Value{}, more{wantBytesOut: []byte("A1bert\nBer1in\n")}},
*/
{`echo "Albert\nAllan\nAlbraham\nBerlin" | sed s/l/1/g | grep e`,
[]Value{}, more{wantBytesOut: []byte("A1bert\nBer1in\n")}},
// Pure channel pipeline
{`put 233 42 19 | each [x]{+ $x 10}`, strs("243", "52", "29"), nomore},
// TODO: Add a useful hybrid pipeline sample