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

"cmd | grep --quiet" introduces race condition #1109

Open
2 tasks done
gozdal opened this issue Feb 7, 2018 · 8 comments
Open
2 tasks done

"cmd | grep --quiet" introduces race condition #1109

gozdal opened this issue Feb 7, 2018 · 8 comments

Comments

@gozdal
Copy link

gozdal commented Feb 7, 2018

Here's a snippet or screenshot that shows the problem:

This script should be endless loop, but after some iterations it'll exit with an error (SIGPIPE delivered). grep --quiet closes input pipe when it matches pattern, while command on the left hand side will exit with an error trying to write to a closed pipe:

#!/bin/bash

set -euo pipefail

count=1

while echo -e "foo\\nbar\\nbaz\\n" | grep --quiet foo; do
    echo "$count"
    count=$((count+1))
done

Here's what shellcheck currently says:

Nothing

Here's what I wanted or expected to see:

Using "cmd | grep --quiet" with "set -o pipefail" introduces race condition, which may cause the script to fail. use "cmd | grep --count" if possible.

@rdebath
Copy link

rdebath commented Feb 14, 2018

Shellcheck doesn't run your code so if you turn pipefail off before you get to the grep it can't tell.
AIUI it also scans the script from top to bottom so if you're using functions it may not have seen the pipefail yet.

Now for your simple example, without any flow control, it could work. But for consistency you would probably have to forbid pipefail being left on and so throw a "You seem to have left pipefail active on line N, this causes unexpected behaviour" if you get to a condition or other flow control (eg: return).

@koalaman
Copy link
Owner

This is a very interesting issue that ShellCheck should be warning about.

Yes, it's a mathematically undecidable problem, but that's easily solved through the magic of false positives :P

@gozdal
Copy link
Author

gozdal commented May 11, 2018

Another fail scenarios: grep --max-count or head on the right hand side of a pipe.

@pothos
Copy link

pothos commented Jul 4, 2022

Similar issue is #665

@mgutt
Copy link

mgutt commented Apr 18, 2023

I really like to see this check as I faced this issue, too:
https://unix.stackexchange.com/questions/743260/has-grep-quiet-a-bug-with-its-exit-status

use "cmd | grep --count" if possible.

If cmd returns 1 million lines of text and the first line contains the match, this will be really slow compared to --quiet. Instead this could be a solution:

# treat SIGPIPE as true
cmd | grep --quiet foo || [[ $? -eq 141 ]]

Another possible solution is not to use pipefail at all or only selectively as mentioned here:

https://mywiki.wooledge.org/BashPitfalls#pipefail

If you know that each part of a pipeline beyond the first will consume all their input, pipefail is safe and often a good choice, but it should not be enabled globally, only enabled for pipelines where it makes sense, and disabled afterwards.

At the moment I tend to remove pipefail. Before I used it, I never had any problems and now I have to think about race conditions in pipes...

@felipecrs
Copy link
Contributor

I would suggest to close this as duplicate of #665.

@pothos
Copy link

pothos commented Aug 1, 2023

FYI, I tend to always use … | { grep ARG || true ; } | … nowadays and the same for … | { cmd || true ; } | head. Suggesting this would be nice even without detecting that -o pipefail is set because it might be that this script gets sourced and the shell already has -o pipefail set.

@mgutt
Copy link

mgutt commented Aug 2, 2023

FYI, I tend to always use … | { grep ARG || true ; } | … nowadays and the same for … | { cmd || true ; } | head. Suggesting this would be nice even without detecting that -o pipefail is set because it might be that this script gets sourced and the shell already has -o pipefail set.

Covering all the exceptions caused by pipefail is much more than checking only the grep commands. For me there are only two options left:

  • never set pipefail
  • people sourcing my scripts with pipefail have to learn their lesson on their own

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants