Skip to content

Allocations in pre_exec #54

@purplesyringa

Description

@purplesyringa

.pre_exec(move || {
cg.add_task(CgroupPid::from(std::process::id() as u64))
.map_err(Error::other)
})

This snippet is invalid in multi-threaded programs, which AFAIK cryptpilot is. The problem is due to two factors:

  1. Error::other always allocates (see Document Error::{new,other} as to be avoided in pre_exec rust-lang/rust#148971).
  2. Cgroup::add_task allocates internally, since it needs to write to a file and do other stuff.

Allocating in pre_exec is bad because forking while the allocator lock is held by another thread can cause deadlocks, since there's no one left to unlock the mutex. I do not believe this is UB per se, but it seems less than ideal still. glibc and other modern libc offer protections against that, as far as I'm aware, but this isn't POSIX-compliant and should preferably be avoided.

Your options are:

  1. Since this function is only run in tests, you could probably update test_volume_base to use a single-threaded runtime, which might resolve the problem. I'm not sure if this works, but it's worth trying.
  2. A generally correct solution would be to make a thread in the parent process responsible for adding the process to the cgroup, and communicate with that thread from the child process via IPC (say, pipes).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions