Skip to content

Commit

Permalink
Make the intermediate process wait conditionally, only for exec
Browse files Browse the repository at this point in the history
Now the intermediate process waits for the main process only
when exec is called, not when the container is created. This
prevents it from waiting for created contianers, for which
exec might not be called.
This also fixes some formatting issues
  • Loading branch information
YJDoc2 authored and utam0k committed Sep 1, 2022
1 parent 3de7458 commit aa5d9c3
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 6 deletions.
8 changes: 6 additions & 2 deletions crates/libcontainer/src/container/builder_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ pub(super) struct ContainerBuilderImpl<'a> {
pub container: Option<Container>,
/// File descriptos preserved/passed to the container init process.
pub preserve_fds: i32,
/// Denotes if the builder is for the exec call or not
pub is_exec_builder: bool,
}

impl<'a> ContainerBuilderImpl<'a> {
Expand Down Expand Up @@ -122,8 +124,10 @@ impl<'a> ContainerBuilderImpl<'a> {
cgroup_manager: cmanager,
};

let (intermediate, init_pid) =
process::container_main_process::container_main_process(&container_args)?;
let (intermediate, init_pid) = process::container_main_process::container_main_process(
&container_args,
self.is_exec_builder,
)?;

// if file to write the pid to is specified, write pid of the child
if let Some(pid_file) = &self.pid_file {
Expand Down
1 change: 1 addition & 0 deletions crates/libcontainer/src/container/init_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ impl<'a> InitContainerBuilder<'a> {
notify_path,
container: Some(container.clone()),
preserve_fds: self.base.preserve_fds,
is_exec_builder: false,
};

builder_impl.create()?;
Expand Down
1 change: 1 addition & 0 deletions crates/libcontainer/src/container/tenant_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ impl<'a> TenantContainerBuilder<'a> {
notify_path: notify_path.clone(),
container: None,
preserve_fds: self.base.preserve_fds,
is_exec_builder: true,
};

let pid = builder_impl.create()?;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{namespaces::Namespaces, process::channel, process::fork};
use anyhow::{Context, Error, Result};
use libcgroups::common::CgroupManager;
use nix::sys::wait::{waitpid, WaitStatus};
use nix::sys::wait::{waitpid, WaitStatus};
use nix::unistd::{Gid, Pid, Uid};
use oci_spec::runtime::{LinuxNamespaceType, LinuxResources};
use procfs::process::Process;
Expand All @@ -15,6 +15,7 @@ pub fn container_intermediate_process(
intermediate_chan: &mut (channel::IntermediateSender, channel::IntermediateReceiver),
init_chan: &mut (channel::InitSender, channel::InitReceiver),
main_sender: &mut channel::MainSender,
wait: bool,
) -> Result<WaitStatus> {
let (inter_sender, inter_receiver) = intermediate_chan;
let (init_sender, init_receiver) = init_chan;
Expand Down Expand Up @@ -117,7 +118,13 @@ pub fn container_intermediate_process(
.close()
.context("failed to close unused init sender")?;

Ok(waitpid(pid, None)?)
if wait {
Ok(waitpid(pid, None)?)
} else {
// we don't actually want to wait, so
// pid and status doesn't really matter
Ok(WaitStatus::Exited(Pid::from_raw(0), 0))
}
}

fn apply_cgroups<C: CgroupManager + ?Sized>(
Expand Down
5 changes: 3 additions & 2 deletions crates/libcontainer/src/process/container_main_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ use nix::{
socket::{self, UnixAddr},
wait::WaitStatus,
},
unistd::{self,Pid},
unistd::{self, Pid},
};
use oci_spec::runtime;
use std::{io::IoSlice, path::Path};

pub fn container_main_process(container_args: &ContainerArgs) -> Result<(Pid, Pid)> {
pub fn container_main_process(container_args: &ContainerArgs, wait: bool) -> Result<(Pid, Pid)> {
// We use a set of channels to communicate between parent and child process.
// Each channel is uni-directional. Because we will pass these channel to
// forked process, we have to be deligent about closing any unused channel.
Expand All @@ -31,6 +31,7 @@ pub fn container_main_process(container_args: &ContainerArgs) -> Result<(Pid, Pi
inter_chan,
init_chan,
main_sender,
wait,
)?;
match t {
WaitStatus::Exited(_, s) => Ok(s),
Expand Down

0 comments on commit aa5d9c3

Please sign in to comment.