mvdan / sh

A shell parser, formatter, and interpreter with bash support; includes shfmt
https://pkg.go.dev/mvdan.cc/sh/v3
BSD 3-Clause "New" or "Revised" License
7.21k stars 341 forks source link

reading from file redirection in while loops waits indefinitely #1099

Open andrewimeson opened 1 week ago

andrewimeson commented 1 week ago

In v3.9.0 of the gosh interpreter there is an issue with reading from file or file substitutions in while loops. The script just hangs when attempting to read from a file or file substitution.

This did work recently. I haven't narrowed down the exact version it broke in yet. I discovered this through it breaking in my Taskfile usage.

Examples

Works: Reading from pipe

#!/usr/bin/env gosh
set -x

echo "hi pipe" | while read -r line; do
    echo "$line"
done

echo "done"

Hangs: Reading from file

#!/usr/bin/env gosh
set -x

echo "hi file" > hi.txt

while read -r line; do
    echo "$line"
done < hi.txt

echo "done"

results in this hanging forever

$ ./while_issue.sh
+ echo 'hi file'
+ read '-r line'
+ echo 'hi file'
hi file
+ read '-r line'

Hangs: Reading from file substitution

#!/usr/bin/env gosh
set -x

while read -r line; do
    echo "$line"
done < <(echo "hi file substitution")

echo done

results in this hanging forever

$ ./while_issue.sh
+ read '-r line'
+ echo 'hi file substitution'
+ echo 'hi file substitution'
hi file substitution
+ read '-r line'
andrewimeson commented 1 week ago

Confirmed that it broke in 3.9.0. It works in 3.8.0.

andrewimeson commented 1 week ago

With git bisect I've determined that #1066 / a89b0be5ed0a8a3f367a5dc21326d7a39ab5c26f is the commit that broke it.

andrewimeson commented 1 day ago

I tried to write a test that would fail, but I can't seem to get a go test (or a CI run in my fork) to fail with my changes (still fails/hangs with the gosh interpreter). It does fail the CI run for Windows with "TODO: support process substitution on Windows," but I don't care about Windows 😀

diff --git a/interp/interp_test.go b/interp/interp_test.go
index 8b535ce3..4a4e1f2a 100644
--- a/interp/interp_test.go
+++ b/interp/interp_test.go
@@ -2905,6 +2905,10 @@ done <<< 2`,
        "while read a; do echo $a; GOSH_CMD=exec_ok $GOSH_PROG; done <<EOF\na\nb\nc\nEOF",
        "a\nexec ok\nb\nexec ok\nc\nexec ok\n",
    },
+   {
+       "while read a; do echo $a; done < <(echo x)",
+       "x\n",
+   },
    // TODO: our final exit status here isn't right.
    // {
    //  "while read a; do echo $a; GOSH_CMD=exec_fail $GOSH_PROG; done <<< 'a\nb\nc'",
$ cd interp/
$ go test
PASS
ok      mvdan.cc/sh/v3/interp   2.808s
andrewimeson commented 1 day ago

Here's a better test that won't run into Windows compatibility issues. I've based off of 3.8.0 and confirmed it passes in CI and it fails in CI for master.

diff --git a/interp/interp_test.go b/interp/interp_test.go
index 8b535ce3..877d8c7a 100644
--- a/interp/interp_test.go
+++ b/interp/interp_test.go
@@ -2905,6 +2905,10 @@ done <<< 2`,
        "while read a; do echo $a; GOSH_CMD=exec_ok $GOSH_PROG; done <<EOF\na\nb\nc\nEOF",
        "a\nexec ok\nb\nexec ok\nc\nexec ok\n",
    },
+   {
+       "echo x > f; while read a; do echo $a; done < f",
+       "x\n",
+   },
    // TODO: our final exit status here isn't right.
    // {
    //  "while read a; do echo $a; GOSH_CMD=exec_fail $GOSH_PROG; done <<< 'a\nb\nc'",

No idea how to solve the actual bug.