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

Ignoring signals fails #57

Closed
mkroening opened this issue Jun 29, 2023 · 5 comments
Closed

Ignoring signals fails #57

mkroening opened this issue Jun 29, 2023 · 5 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@mkroening
Copy link
Member

mkroening commented Jun 29, 2023

On Linux, when the Rust runtime initializes, it resets the SIGPIPE handler to ignore. (std/src/sys/unix/mod.rs#L59-L67, std/src/sys/unix/mod.rs#L164-L195):

    // By default, some platforms will send a *signal* when an EPIPE error
    // would otherwise be delivered. This runtime doesn't install a SIGPIPE
    // handler, causing it to kill the program, which isn't exactly what we
    // want!
    //
    // Hence, we set SIGPIPE to ignore when the program starts up in order
    // to prevent this problem. Add `#[unix_sigpipe = "..."]` above `fn main()` to
    // alter this behavior.

What Rust does is equivalent to the following C program:

#include <assert.h>
#include <signal.h>

int main(int argc, char *argv[]) {
    void (*res)(int) = signal(SIGPIPE, SIG_IGN);
    assert(res != SIG_ERR);

    return 0;
}
  • On Linux, this succeeds.
  • On Unikraft, with uksignal, this succeeds.
  • On Unikraft, with musl, this fails.
[...]
Assertion failed: res != SIG_ERR (/home/kroening/devel/unikraft/apps/app-helloworld/main.c: main: 6)
[    0.105735] CRIT: [libmusl] <abort.c @    7> abort called

Returning a success value without doing anything would resolve this problem.

@mkroening mkroening added this to Rust Jun 29, 2023
@mkroening mkroening moved this to Todo in Rust Jun 29, 2023
@razvand razvand added this to the v0.14.0 (Prometheus) milestone Jul 5, 2023
@razvand razvand added bug Something isn't working enhancement New feature or request labels Jul 5, 2023
@eduardvintila
Copy link
Member

eduardvintila commented Jul 8, 2023

Hey, @mkroening. I have looked into this issue and found out the problem lies in a mismatch between sigaction structs of Unikraft and the libc ones. Fortunately, we already have @andreittr's PR which solves this problem 👍🏻 Please give it a try and let us know if this issue still persists - I've tried it myself on your example and it works.

@mkroening
Copy link
Member Author

Hi @eduardvintila, thanks for looking into this! :)

Unfortunately, this does not solve the issue for me. I can still produce the issue with the C code above. Can you reproduce the issue with an unpatched Unikraft?

My Kraftfile looks as follows:

specification: v0.5

unikraft:
  version: stable

targets:
  - name: default
    architecture: x86_64
    platform: kvm

libraries:
  musl: stable

Before and after applying the patch and rebuilding with --no-pull, signal returns SIG_ERR.

Note that I circumvented the issue for now—at least for Rust startup. I opted out of using signal for Unikraft (rust-lang/rust@9ef2572), but it would be great if we would not have to.

@mkroening mkroening moved this from Todo to In Progress in Rust Jul 10, 2023
@eduardvintila
Copy link
Member

eduardvintila commented Jul 27, 2023

Hey, @mkroening! Sorry for taking so much time to get back with a response...

The PR I have mentioned is now upstream. Please try to run everything while on the staging (not stable) branch. Do you have the uksignal library selected? You must include it in order for your example to work - the issue is indeed present when it's not enabled :)

@mkroening
Copy link
Member Author

Thanks for your reply! I can confirm it works.

The hint with uksignal was especially helpful. I found it surprising that CONFIG_LIBMUSL_SIGNAL does not enable CONFIG_LIBUKSIGNAL, but you instead have to do that manually.

Do you think that behavior should be changed, and I should open an issue, or is this intended?

@github-project-automation github-project-automation bot moved this from In Progress to Done in Rust Jul 27, 2023
@eduardvintila
Copy link
Member

eduardvintila commented Jul 27, 2023

@mkroening, you make a very good point - probably the reason why this was not the case is that uksignal used to be broken a few releases back. Even though we still don't have a complete implementation, it's functional now. No need for an issue, I think. I'll directly make a PR on this. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

3 participants