Remove O_NONBLOCK.

[analytically]

@mxpv

[mxpv]

Looks like this affects both integration and formatting checks. Latest main is green.

[jokemanfire]

@analytically It's may not CI’s problem you should open stdin write side to make sure read side will not block
code like this

  if !init.stdio.stdin.is_empty() {
            let stdin_clone = init.stdio.stdin.clone();
            let stdin_w = init.stdin.clone();
            // Open the write side in advance to make sure read side will not block,
            // open it in another thread otherwise it will block too.
            tokio::spawn(async move {
                if let Ok(stdin_w_file) = OpenOptions::new()
                    .write(true)
                    // .custom_flags(libc::O_NONBLOCK)
                    .open(stdin_clone.as_str())
                    .await
                {
                    let mut lock_guard = stdin_w.lock().unwrap();
                    *lock_guard = Some(stdin_w_file);
                }
            });
        }

And because the set is sync so I have to add a timeout to make sure the open write task have been falled into tokio::time::sleep(Duration::from_secs(1)).await;
before

let resp = init
            .lifecycle
            .runtime
            .create(&id, bundle, Some(&create_opts))
            .await;

We may should change io set to async trait next step @mxpv

[analytically]

So in my setup, the parent process (concourse-worker) has already opened the write end of the FIFO. Concourse resource containers expect to be able to block on stdin reads when waiting for input.

[analytically]

Just removing NONBLOCK makes rust-extensions work flawlessly with Concourse CI (https://concourse-ci.org/). I overwrite the bundled containerd-shim-runc-v2-rs and am able to run > 500 containers on a single host, with the Go version I cannot go more than 180.

[jokemanfire]

So in my setup, the parent process (concourse-worker) has already opened the write end of the FIFO. Concourse resource containers expect to be able to block on stdin reads when waiting for input.

In the contaienrd's CI the write side may not open, I've pasted your change and run it in CI , Open write side in shim side can pass this CI. If your's recommand want to pass this CI?

[analytically]

In production Concourse usage, the write side would be opened by shim and the read would work properly. But in CI where there's no shim/write side, the NONBLOCK flag allows the tests to proceed without hanging.

[analytically]

// Add NONBLOCK only in CI environment
if std::env::var("CI").is_ok() {
  options.custom_flags(libc::O_NONBLOCK);
}

[jokemanfire]

In production Concourse usage, the write side would be opened by shim and the read would work properly. But in CI where there's no shim/write side, the NONBLOCK flag allows the tests to proceed without hanging.

if it means you want to do a sync in stdin write and read. Blocking in stdin's read may not greate.
If you means the runc's process should be block on until the stdin write to be opened . I think it should be think more about it because there should be many design considerations involved here.

[analytically]

Maybe make it configurable?

#[derive(Debug, Clone)]
pub struct IOOption {
    pub open_stdin: bool,
    pub open_stdout: bool,
    pub open_stderr: bool,
    pub stdin_nonblock: bool,  // New configuration option
}

impl Default for IOOption {
    fn default() -> Self {
        Self {
            open_stdin: true,
            open_stdout: true,
            open_stderr: true,
            stdin_nonblock: false,  // Default to blocking behavior
        }
    }
}

Then in FIFO:

impl FIFO {
    fn set(&self, cmd: &mut Command, opts: &IOOption) -> Result<()> {
        if let Some(path) = self.stdin.as_ref() {
            let mut options = OpenOptions::new().read(true);
            if opts.stdin_nonblock {
                options.custom_flags(libc::O_NONBLOCK);
            }
            let stdin = options.open(path)?;
            cmd.stdin(stdin);
        }
        // ...
    }
}

[jokemanfire]

Just removing NONBLOCK makes rust-extensions work flawlessly with Concourse CI (https://concourse-ci.org/). I overwrite the bundled containerd-shim-runc-v2-rs and am able to run > 500 containers on a single host, with the Go version I cannot go more than 180.

Maybe make it configurable?

#[derive(Debug, Clone)]
pub struct IOOption {
    pub open_stdin: bool,
    pub open_stdout: bool,
    pub open_stderr: bool,
    pub stdin_nonblock: bool,  // New configuration option
}

impl Default for IOOption {
    fn default() -> Self {
        Self {
            open_stdin: true,
            open_stdout: true,
            open_stderr: true,
            stdin_nonblock: false,  // Default to blocking behavior
        }
    }
}

Then in FIFO:

impl FIFO {
    fn set(&self, cmd: &mut Command, opts: &IOOption) -> Result<()> {
        if let Some(path) = self.stdin.as_ref() {
            let mut options = OpenOptions::new().read(true);
            if opts.stdin_nonblock {
                options.custom_flags(libc::O_NONBLOCK);
            }
            let stdin = options.open(path)?;
            cmd.stdin(stdin);
        }
        // ...
    }
}

I think that's will be great , and read the env while set the IOOption. How about change the env's name "CI" to other name which can be more easily understood?

[analytically]

The env name "CI" is standard for a lot of CI tools, see https://github.com/actions/runner/blob/fde5227fbfe9c61b7861cc959ebbbba62af4754b/src/Runner.Sdk/ProcessInvoker.cs#L284

[jokemanfire]

The env name "CI" is standard for a lot of CI tools, see https://github.com/actions/runner/blob/fde5227fbfe9c61b7861cc959ebbbba62af4754b/src/Runner.Sdk/ProcessInvoker.cs#L284

That's well, thanks.

[jokemanfire]

The env name "CI" is standard for a lot of CI tools, see https://github.com/actions/runner/blob/fde5227fbfe9c61b7861cc959ebbbba62af4754b/src/Runner.Sdk/ProcessInvoker.cs#L284

I've changed the IO set for Blocking IO's Time consuming issue, may need a rebase.