grep hangs indefinitely on FIFO files

[firasuke]

Hey there,

Thank you for your time and effort on toybox!

I came across the following behavior when using toybox grep. It appears to freeze or stall indefinitely when the file is a FIFO (named pipe).

To reproduce this issue, simply create a fifo file in a directory:

mkfifo -m 600 namedPipe

Then attempt to run toybox grep -rnw . -e "some expression" (in the directory where the namedPipe exists in or in one of its subdirectories) and it will freeze indefinitely.

Please note that both GNU grep and ugrep do not suffer from this issue (it is possible that they are skipping these special files entirely by default?).

Thanks in advance!

[landley]

On 10/17/24 15:03, Firas Khalil Khana wrote: `toybox grep` appears to freeze or stall indefinitely when the file is a FIFO (named pipe).

Hmmm, according to strace, debian's grep is opening the file O_RDONLY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW. I noticed the O_NOFOLLOW because it breaks grepping a cp -s tree of symlinks, and I don't intend to copy that. (The -r vs -R logic is already distinguishing DIRECTORY symlinks, file symlinks should really act like the file they point to and refusing to do that is breakage unless Posix specified something weird recently that I missed?) O_RDONLY we're already doing (it's 0, strace is hallucinating it here because neither O_RDWR nor O_WRONLY were specified, don't ask me what happens if you OR those together...) O_NOCTTY seems fine, my question with O_NONBLOCK is what happens if you're reading from a network filesystem that takes a bit to talk to the other end, or you're on an external spinny media disk that needs to spin up? How much block counts as block? (I'm aware it's really easy for NFS to get stuck in D state, maybe this is why?) *shrug* For now I can add the two new flags and a test, but heads up for Elliott that this _might_ cause weirdness out in the giant surface area of android devices? (Probably won't, but...) Commit 4073e77

[enh-google]

On Sun, Oct 20, 2024 at 11:50 AM Rob Landley ***@***.***> wrote:

On 10/17/24 15:03, Firas Khalil Khana wrote: > `toybox grep` appears to freeze or stall indefinitely when the file > is a FIFO (named pipe). Hmmm, according to strace, debian's grep is opening the file O_RDONLY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW. I noticed the O_NOFOLLOW because it breaks grepping a cp -s tree of symlinks, and I don't intend to copy that. (The -r vs -R logic is already distinguishing DIRECTORY symlinks, file symlinks should really act like the file they point to and refusing to do that is breakage unless Posix specified something weird recently that I missed?) O_RDONLY we're already doing (it's 0, strace is hallucinating it here because neither O_RDWR nor O_WRONLY were specified, don't ask me what happens if you OR those together...) O_NOCTTY seems fine, my question with O_NONBLOCK is what happens if you're reading from a network filesystem that takes a bit to talk to the other end, or you're on an external spinny media disk that needs to spin up? How much block counts as block? (I'm aware it's really easy for NFS to get stuck in D state, maybe this is why?) *shrug* For now I can add the two new flags and a test, but heads up for Elliott that this _might_ cause weirdness out in the giant surface area of android devices? (Probably won't, but...)

(passed the toybox tests in presubmit at least. i'll let you know if i hear anything now it's merged though.)

…

Commit 4073e77 — Reply to this email directly, view it on GitHub <#523 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMVLEWDPAE6ZBLPPNCRNEWTZ4PGLPAVCNFSM6AAAAABQEP6TWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRVGA2TQOBYGE> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[landley]

On 10/22/24 11:11, enh-google wrote: > *shrug* For now I can add the two new flags and a test, but heads up for > Elliott that this _might_ cause weirdness out in the giant surface area > of android devices? (Probably won't, but...) (passed the toybox tests in presubmit at least. i'll let you know if i hear anything now it's merged though.)

With a little further tought: the nonblock flag is on the open but not on the read, and the directory entries have their own separate open() and readdir() before we open the file itself, so the granularity is correct for not waiting for connect() or similar to find another end, so this was probably the correct fix. (I wonder if I can close an issue via email? I can wget the patch to apply locally and reply to notifications like this. It would be nice if I could close issues this way too, for when they take away my web login in november...) Rob

[enh-google]

On Tue, Oct 22, 2024 at 3:21 PM Rob Landley ***@***.***> wrote:

On 10/22/24 11:11, enh-google wrote: >> *shrug* For now I can add the two new flags and a test, but heads up for >> Elliott that this _might_ cause weirdness out in the giant surface area >> of android devices? (Probably won't, but...) > > (passed the toybox tests in presubmit at least. i'll let you know if i hear > anything now it's merged though.) With a little further tought: the nonblock flag is on the open but not on the read, and the directory entries have their own separate open() and readdir() before we open the file itself, so the granularity is correct for not waiting for connect() or similar to find another end, so this was probably the correct fix. (I wonder if I can close an issue via email? I can wget the patch to apply locally and reply to notifications like this. It would be nice if I could close issues this way too, for when they take away my web login in november...)

i don't know about that, but you can use "Fixes #<bug number>" to auto-close on push.

…

Rob — Reply to this email directly, view it on GitHub <#523 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMVLEWBL2R5JHRMXDXEAJXDZ42QS5AVCNFSM6AAAAABQEP6TWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZQGA3DMNBQGQ> . You are receiving this because you commented.Message ID: ***@***.***>