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

scubainit ignores SIGPIPE #254

Closed
haboustak opened this issue May 11, 2024 · 4 comments · Fixed by #255
Closed

scubainit ignores SIGPIPE #254

haboustak opened this issue May 11, 2024 · 4 comments · Fixed by #255
Assignees
Labels
bug scubainit Relating to the scubainit binary which runs at container startup

Comments

@haboustak
Copy link
Contributor

haboustak commented May 11, 2024

The new scubainit included in version 2.13.0 ignores SIGPIPE, which is then inherited by any process that scuba runs. This behavior is included as a side-effect of the Rust entrypoint. See rust-lang/rust #62569

Here's a test case that was shared with me recently and demonstrates the behavior change from 2.12.0 to 2.13.0:

$ scuba --image debian:12 sh -c "yes | echo abcd"
abcd
yes: standard output: Broken pipe

Also, to show that SIGPIPE is ignored in 2.13.0:

$ scuba --image debian:12 cat /proc/1/status | grep "Sig"
SigQ:	1/31475
SigPnd:	0000000000000000
SigBlk:	0000000000000000
SigIgn:	0000000000001000
SigCgt:	0000000000010002

I think the correct behavior for scuba is to reset SIGPIPE to SIG_DFL.

@JonathonReinhart
Copy link
Owner

I believe this is handled by #253, but I haven't looked at it yet.

@haboustak
Copy link
Contributor Author

Ah yes. I came here looking for an issue and not a PR. I’d imagine the compiler flag will fix it.

@JonathonReinhart
Copy link
Owner

If I understand the problem statement correctly:

  • The default disposition of SIGPIPE is Term: signal(7)
  • From pipe(7):

    If all file descriptors referring to the read end of a pipe have been closed, then a write(2) will cause a SIGPIPE signal to be generated for the calling process. If the calling process is ignoring this signal, then write(2) fails with the error EPIPE.

  • So normally, yes is terminated by SIGPIPE with no error output (before write returns) when the reader closes the pipe.
  • But with scubainit ignoring SIGPIPE (due to Rust startup code), and yes inheriting it, yes instead sees write return with EPIPE, resulting in an error message before termination.

Please correct me if I got any of that wrong.

@JonathonReinhart
Copy link
Owner

demo.sh:

#!/bin/bash
vers=(2.12.0 2.13.0)
for ver in ${vers[@]}; do
    echo "======== Scuba ${ver} ========"
    venv="scuba-${ver}"
    python3 -m venv $venv || exit $?
    ${venv}/bin/pip install "scuba==${ver}" || exit $?
    ${venv}/bin/scuba --version
    ${venv}/bin/scuba --image debian:12 sh -c "yes | echo abcd"
done

Output:

$ ./demo.sh
======== Scuba 2.12.0 ========
Requirement already satisfied: scuba==2.12.0 in ./scuba-2.12.0/lib/python3.11/site-packages (2.12.0)
Requirement already satisfied: PyYAML in ./scuba-2.12.0/lib/python3.11/site-packages (from scuba==2.12.0) (6.0.1)
scuba 2.12.0
abcd
======== Scuba 2.13.0 ========
Requirement already satisfied: scuba==2.13.0 in ./scuba-2.13.0/lib/python3.11/site-packages (2.13.0)
Requirement already satisfied: PyYAML in ./scuba-2.13.0/lib/python3.11/site-packages (from scuba==2.13.0) (6.0.1)
scuba 2.13.0
abcd
yes: standard output: Broken pipe

This matches the problem described above.

As does this:

$ strace -o yes.strace yes | echo abcd

yes.strace:

...
write(1, "y\ny\ny\ny\ny\ny\ny\ny\ny\ny\ny\ny\ny\ny\ny\ny\n"..., 8192) = -1 EPIPE (Broken pipe)
--- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=693228, si_uid=1000} ---
+++ killed by SIGPIPE +++

JonathonReinhart added a commit that referenced this issue May 20, 2024
Rust pre-main code may change the SIGPIPE disposition to ignore:
* rust-lang/rust#62569
* rust-lang/rust#97889

We could use the nightly compiler flag -Zon-broken-pipe=inherit to
disable this behavior. Instead, we take the simpler route and restore
the default disposition ourselves.

Fixes #254
JonathonReinhart added a commit that referenced this issue May 20, 2024
Rust pre-main code may change the SIGPIPE disposition to ignore:
* rust-lang/rust#62569
* rust-lang/rust#97889

We could use the nightly compiler flag -Zon-broken-pipe=inherit to
disable this behavior. Instead, we take the simpler route and restore
the default disposition ourselves.

Fixes #254
JonathonReinhart added a commit that referenced this issue May 22, 2024
Rust pre-main code may change the SIGPIPE disposition to ignore:
* rust-lang/rust#62569
* rust-lang/rust#97889

We could use the nightly compiler flag -Zon-broken-pipe=inherit to
disable this behavior. Instead, we take the simpler route and restore
the default disposition ourselves.

Fixes #254
JonathonReinhart added a commit that referenced this issue May 29, 2024
Rust pre-main code may change the SIGPIPE disposition to ignore:
* rust-lang/rust#62569
* rust-lang/rust#97889

We could use the nightly compiler flag -Zon-broken-pipe=inherit to
disable this behavior. Instead, we take the simpler route and restore
the default disposition ourselves.

Fixes #254
@JonathonReinhart JonathonReinhart added bug scubainit Relating to the scubainit binary which runs at container startup labels May 29, 2024
@JonathonReinhart JonathonReinhart self-assigned this May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug scubainit Relating to the scubainit binary which runs at container startup
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants