Skip to content

fork or posix_spawn return EAGAIN, should return ENOSYS #14124

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

Closed
ijackson opened this issue May 7, 2021 · 1 comment · Fixed by #14173
Closed

fork or posix_spawn return EAGAIN, should return ENOSYS #14124

ijackson opened this issue May 7, 2021 · 1 comment · Fixed by #14173

Comments

@ijackson
Copy link
Contributor

ijackson commented May 7, 2021

Hi. I've come here to try to upstream a bug report about emscripten's behaviour which we experienced in the Rust CI. I hope this is appreciated.

tl;dr: I think, based on the behaviour of an emscripten test which runs in the Rust CI, and cursory inspection of library.js, that either fork or posix_spawn sets errno to EAGAIN/EWOULDBLOCK. IMO this is not correct. The correct response is ENOSYS.

I had a look through the emscripten issues and MRs, and found #3819 where posix_spawn was stubbed out, and based on that I think the fix is to edit library.js. I can prepare a drive-by MR for that if you like.

More detailed information

As I say I am prompted by a failing test case in the Rust CI. The Rust code looks like this:

    let got = catch_unwind(|| {
        let mut c = Command::new("echo");
        c.arg("hi");
        unsafe {
            c.pre_exec(|| panic!("{}", "crash now!"));
        }
        let st = c.status().expect("failed to get command status");
        dbg!(st);
        st
    });
    dbg!(&got);
    let status = got.expect("panic unexpectedly propagated");
    dbg!(status);
    let signal = status.signal().expect("expected child process to die of signal");
    assert!(signal == libc::SIGABRT || signal == libc::SIGILL);

This failed:
rust-lang/rust#85032 (comment)

The error message includes this:

  thread 'main' panicked at 'failed to get command status: Os { code: 6, kind: WouldBlock, message: "Resource temporarily unavailable" }', library/std/src/sys/unix/process/process_unix/tests.rs:47:29

Evidently Command:status() gave an error with ErrorKind::WouldBlock. That is wrong. It should be Unsupported. Rust has its own error number mapping. Here WouldBlock came from EAGAIN or EWOULDBLOCK.

The code in the implementation of Command is not entirely trivial. I was working on it as part of rust-lang/rust#81858 so I'm reasonably familiar with it. I won't walk you through it, but basically, it first tries posix_spawn and then falls back to fork.

Thanks for your attention. Let me know if you want me to make an MR to change this in emscripten.

@ijackson ijackson changed the title fork or posix_spawn return EAGAIN, should return EWOULDBLOCK fork or posix_spawn return EAGAIN, should return ENOSYS May 7, 2021
@kripken
Copy link
Member

kripken commented May 7, 2021

Yes, it does look like we return EAGAIN for those:

emscripten/src/library.js

Lines 166 to 178 in b8724b8

// fork, spawn, etc. all return an error as we don't support multiple
// processes.
fork__deps: ['$setErrNo'],
fork__sig: 'i',
fork: function() {
// pid_t fork(void);
// http://pubs.opengroup.org/onlinepubs/000095399/functions/fork.html
// We don't support multiple processes.
setErrNo({{{ cDefine('EAGAIN') }}});
return -1;
},
vfork: 'fork',
posix_spawn: 'fork',

Reading the manpage, I think I agree that ENOSYS would be more accurate (as EAGAIN means "hit the limit on the number of threads", or some other more temporary limit, and ENOSYS is a permanent limit). PR would be welcome!

ijackson added a commit to ijackson/emscripten that referenced this issue May 13, 2021
This is more correct.  Trying again will not help.

Fixes: emscripten-core#14124
sbc100 pushed a commit that referenced this issue May 14, 2021
This is more correct.  Trying again will not help.

Fixes: #14124
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

Successfully merging a pull request may close this issue.

2 participants