Input & Driver: Fix zombie processes on Unix (#39)

Linux/Unix requires that processes be waited, which is unfortunate as Windows lets us abandon them to the murderous whims of the OS. This PR adds Unix-specific behaviour to send a SIGINT before waiting on the process, and adds an additional thread per call for asset disposal on all platforms.

Closes #38.

---

* Close processes by SIGINT and wait on Unix

This seems to remedy the Linux-specific zombie processes. Addition of
nix as a dependency *should* be fine on Windows, since I believe it
compiles to an empty crate.

* Dispose of Tracks on auxiliary thread

This adds a mechanism for the mixer threads to perform potentially expensive deallocation/cleanup outside of the main loop, preventing deadline misses etc. This should make misbehaving `wait`s a bit more friendly.
This commit is contained in:
Kyle Simpson
2021-01-26 20:19:51 +00:00
committed by GitHub
parent a0e905a83f
commit fe2282cfde
7 changed files with 64 additions and 4 deletions

View File

@@ -52,6 +52,10 @@ version = "0.10"
[dependencies.futures] [dependencies.futures]
version = "0.3" version = "0.3"
[dependencies.nix]
version = "0.19"
optional = true
[dependencies.parking_lot] [dependencies.parking_lot]
optional = true optional = true
version = "0.11" version = "0.11"
@@ -133,6 +137,7 @@ driver = [
"byteorder", "byteorder",
"discortp", "discortp",
"flume", "flume",
"nix",
"parking_lot", "parking_lot",
"rand", "rand",
"serenity-voice-model", "serenity-voice-model",

View File

@@ -0,0 +1,18 @@
use super::message::*;
use flume::Receiver;
use tracing::instrument;
/// The mixer's disposal thread is also synchronous, due to tracks,
/// inputs, etc. being based on synchronous I/O.
///
/// The mixer uses this to offload heavy and expensive drop operations
/// to prevent deadline misses.
#[instrument(skip(mix_rx))]
pub(crate) fn runner(mix_rx: Receiver<DisposalMessage>) {
loop {
match mix_rx.recv() {
Err(_) | Ok(DisposalMessage::Poison) => break,
_ => {},
}
}
}

View File

@@ -0,0 +1,9 @@
#![allow(missing_docs)]
use crate::tracks::Track;
pub enum DisposalMessage {
Track(Track),
Poison,
}

View File

@@ -1,13 +1,14 @@
#![allow(missing_docs)] #![allow(missing_docs)]
mod core; mod core;
mod disposal;
mod events; mod events;
mod mixer; mod mixer;
mod udp_rx; mod udp_rx;
mod udp_tx; mod udp_tx;
mod ws; mod ws;
pub use self::{core::*, events::*, mixer::*, udp_rx::*, udp_tx::*, ws::*}; pub use self::{core::*, disposal::*, events::*, mixer::*, udp_rx::*, udp_tx::*, ws::*};
use flume::Sender; use flume::Sender;
use tracing::info; use tracing::info;

View File

@@ -1,4 +1,4 @@
use super::{error::Result, message::*, Config}; use super::{disposal, error::Result, message::*, Config};
use crate::{ use crate::{
constants::*, constants::*,
tracks::{PlayMode, Track}, tracks::{PlayMode, Track},
@@ -28,6 +28,7 @@ pub struct Mixer {
pub config: Config, pub config: Config,
pub conn_active: Option<MixerConnection>, pub conn_active: Option<MixerConnection>,
pub deadline: Instant, pub deadline: Instant,
pub disposer: Sender<DisposalMessage>,
pub encoder: OpusEncoder, pub encoder: OpusEncoder,
pub interconnect: Interconnect, pub interconnect: Interconnect,
pub mix_rx: Receiver<MixerMessage>, pub mix_rx: Receiver<MixerMessage>,
@@ -74,12 +75,17 @@ impl Mixer {
let tracks = Vec::with_capacity(1.max(config.preallocated_tracks)); let tracks = Vec::with_capacity(1.max(config.preallocated_tracks));
// Create an object disposal thread here.
let (disposer, disposal_rx) = flume::unbounded();
std::thread::spawn(move || disposal::runner(disposal_rx));
Self { Self {
async_handle, async_handle,
bitrate, bitrate,
config, config,
conn_active: None, conn_active: None,
deadline: Instant::now(), deadline: Instant::now(),
disposer,
encoder, encoder,
interconnect, interconnect,
mix_rx, mix_rx,
@@ -322,12 +328,13 @@ impl Mixer {
if track.playing.is_done() { if track.playing.is_done() {
let p_state = track.playing(); let p_state = track.playing();
self.tracks.swap_remove(i); let to_drop = self.tracks.swap_remove(i);
to_remove.push(i); to_remove.push(i);
self.fire_event(EventMessage::ChangeState( self.fire_event(EventMessage::ChangeState(
i, i,
TrackStateChange::Mode(p_state), TrackStateChange::Mode(p_state),
))?; ))?;
let _ = self.disposer.send(DisposalMessage::Track(to_drop));
} else { } else {
i += 1; i += 1;
} }
@@ -580,4 +587,6 @@ pub(crate) fn runner(
let mut mixer = Mixer::new(mix_rx, async_handle, interconnect, config); let mut mixer = Mixer::new(mix_rx, async_handle, interconnect, config);
mixer.run(); mixer.run();
let _ = mixer.disposer.send(DisposalMessage::Poison);
} }

View File

@@ -1,5 +1,6 @@
#![allow(missing_docs)] #![allow(missing_docs)]
pub(crate) mod disposal;
pub mod error; pub mod error;
mod events; mod events;
pub mod message; pub mod message;

View File

@@ -5,6 +5,12 @@ use std::{
}; };
use tracing::debug; use tracing::debug;
#[cfg(unix)]
use nix::{
sys::signal::{self, Signal},
unistd::Pid,
};
/// Handle for a child process which ensures that any subprocesses are properly closed /// Handle for a child process which ensures that any subprocesses are properly closed
/// on drop. /// on drop.
#[derive(Debug)] #[derive(Debug)]
@@ -31,7 +37,18 @@ impl Read for ChildContainer {
impl Drop for ChildContainer { impl Drop for ChildContainer {
fn drop(&mut self) { fn drop(&mut self) {
if let Err(e) = self.0.kill() { #[cfg(not(unix))]
let attempt = self.0.kill();
#[cfg(unix)]
let attempt = {
let pid = Pid::from_raw(self.0.id() as i32);
let _ = signal::kill(pid, Signal::SIGINT);
self.0.wait()
};
if let Err(e) = attempt {
debug!("Error awaiting child process: {:?}", e); debug!("Error awaiting child process: {:?}", e);
} }
} }