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

bats waits for bg processes in setup_file even when FD3 is closed #530

Closed
jbriales opened this issue Dec 29, 2021 · 5 comments · Fixed by #535
Closed

bats waits for bg processes in setup_file even when FD3 is closed #530

jbriales opened this issue Dec 29, 2021 · 5 comments · Fixed by #535
Labels
Component: Docs Priority: Medium Wrong or misleading documentation, broken behavior with workaround Type: Bug

Comments

@jbriales
Copy link
Contributor

jbriales commented Dec 29, 2021

I'm following the indications in https://bats-core.readthedocs.io/en/stable/writing-tests.html#file-descriptor-3-read-this-if-bats-hangs and that seems to work when bg processes like sleep 5s 3>- & are launched from the test body or the (per-test) setup function. But when launched from the (per-file) setup_file function, sleep 5s 3>- & is still blocking the bats execution after all tests are done.

To reproduce:

@test "bats does not hang on bg process in setup_file" {
  cd "$BATS_TEST_TMPDIR"

  cat <<EOF >foo.bats
setup_file (){
  sleep 5s 3>- &
}

$(echo @test) "foo" {
  true
}
EOF

  SECONDS=0
  run bats foo.bats
  test $SECONDS -lt 2
}

Is this a known issue? I have seen very recent work around this topic in #525 but not sure if that will apply to this particular issue.

Environment:

  • Bats 1.5.0
  • OS: Fedora 33
  • Bash version: GNU bash, version 5.0.17(1)-release (x86_64-redhat-linux-gnu)
@martin-schulze-vireso
Copy link
Member

I did not investigate setup_file yet but I'd assume the same caveats apply. There are two main culprits: the recently introduced trace functionality relies on FD4 wihch is opened unconditionally, so you might want to close this too. And then there is the FD saving by bash described here, which requires close_non_std_fds from the PR.

In any case the documentation should be updated accordingly.

@martin-schulze-vireso martin-schulze-vireso added Component: Docs Priority: Medium Wrong or misleading documentation, broken behavior with workaround labels Dec 29, 2021
@jbriales
Copy link
Contributor Author

jbriales commented Dec 30, 2021

I gave it a try to address the hanging bg process with the pointers above, on top of latest master.
Neither closing FD4 with 4>&- nor using close_non_std_fds seemed to work.

FYI this is what I tried: jbriales@bc22c13

@jbriales
Copy link
Contributor Author

Follow-up of the above: Not sure why close_non_std_fds would not work.
On another try I got this to work:

(
  for fd in $(ls /proc/$$/fd/); do
    [ $fd -gt 2 ] && exec {fd}<&-
  done
  sleep 2s &>/dev/null &
)

On closer inspection, it seems this minimal set of FDs to close unblocks the bats process for bg processes inside setup_file:

sleep 2s &>/dev/null 3>&- 4>&- 63>&- &

I.e. it seems for some reason I'm not aware of FD63 is also set for setup_file (that did not seem the case when checking in setup or a usual test), and that as well as the others need to be closed/redirected to avoid blocking the test suite.

@martin-schulze-vireso
Copy link
Member

martin-schulze-vireso commented Dec 30, 2021

I already had a short look and could reproduce the issue. I noticed that the close_non_std_fds function says it closed everything but the get_open_fds function still says they are all open afterwards. I assume there is some problem with the eval "exec $fd>&-", which is mainly used for backwards compatibility with older bash versions that don't support exec fd>&-. I'll continue my investigation later.

Those issues with unpredictable pipe duplicating FD numbers are exactly why I introduced this function.

martin-schulze-vireso added a commit to martin-schulze-vireso/bats-core that referenced this issue Jan 4, 2022
martin-schulze-vireso added a commit to martin-schulze-vireso/bats-core that referenced this issue Jan 4, 2022
martin-schulze-vireso added a commit to martin-schulze-vireso/bats-core that referenced this issue Jan 4, 2022
@martin-schulze-vireso
Copy link
Member

Just to wrap up what was happening: The stdout/err output from setup_file was redirected into a process substitution which obviously has to be kept open until the pipe is closed from the producer end. When your background process does not close these fds too, it will keep the process substitution open, which in turn keeps the test suite process open... This was not a problem within tests as they don't redirect into a process substitution but into a file. Hence, I rewrote the setup_file rigging to also redirect into a file and do the translations of the process substitution at later point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Docs Priority: Medium Wrong or misleading documentation, broken behavior with workaround Type: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants