Skip to content

Commit

Permalink
Merge pull request #160 from benfred/freebsd_attach
Browse files Browse the repository at this point in the history
On FreeBSD warn about potential issues before allowing --pid
  • Loading branch information
benfred authored Sep 2, 2019
2 parents 18430c5 + 0a52002 commit 4a490a2
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 15 deletions.
45 changes: 31 additions & 14 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,49 +247,66 @@ impl Config {
error!("Can't get native stack traces with the --nonblocking option. Disabling native.");
config.native = false;
}

#[cfg(target_os="freebsd")]
{
if config.pid.is_some() {
if std::env::var("PYSPY_ALLOW_FREEBSD_ATTACH").is_err() {
eprintln!("On FreeBSD, running py-spy can cause an exception in the profiled process if the process \
is calling 'socket.connect'.");
eprintln!("While this is fixed in recent versions of python, you need to acknowledge the risk here by \
setting an environment variable PYSPY_ALLOW_FREEBSD_ATTACH to run this command.");
eprintln!("\nSee https://github.com/benfred/py-spy/issues/147 for more information");
std::process::exit(-1);
}
}
}
Ok(config)
}
}

#[cfg(test)]
mod tests {
use super::*;
fn split(cmd: &str) -> Vec<String> {
cmd.split_whitespace().map(|x| x.to_owned()).collect()
fn get_config(cmd: &str) -> clap::Result<Config> {
#[cfg(target_os="freebsd")]
std::env::set_var("PYSPY_ALLOW_FREEBSD_ATTACH", "1");
let args: Vec<String> = cmd.split_whitespace().map(|x| x.to_owned()).collect();
Config::from_args(&args)
}

#[test]
fn test_parse_record_args() {
// basic use case
let config = Config::from_args(&split("py-spy record --pid 1234 --output foo")).unwrap();
let config = get_config("py-spy record --pid 1234 --output foo").unwrap();
assert_eq!(config.pid, Some(1234));
assert_eq!(config.filename, Some(String::from("foo")));
assert_eq!(config.format, Some(FileFormat::flamegraph));
assert_eq!(config.command, String::from("record"));

// same command using short versions of everything
let short_config = Config::from_args(&split("py-spy r -p 1234 -o foo")).unwrap();
let short_config = get_config("py-spy r -p 1234 -o foo").unwrap();
assert_eq!(config, short_config);

// missing the --pid argument should fail
assert_eq!(Config::from_args(&split("py-spy record -o foo")).unwrap_err().kind,
assert_eq!(get_config("py-spy record -o foo").unwrap_err().kind,
clap::ErrorKind::MissingRequiredArgument);

// but should work when passed a python program
let program_config = Config::from_args(&split("py-spy r -o foo -- python test.py")).unwrap();
let program_config = get_config("py-spy r -o foo -- python test.py").unwrap();
assert_eq!(program_config.python_program, Some(vec![String::from("python"), String::from("test.py")]));
assert_eq!(program_config.pid, None);

// passing an invalid file format should fail
assert_eq!(Config::from_args(&split("py-spy r -p 1234 -o foo -f unknown")).unwrap_err().kind,
assert_eq!(get_config("py-spy r -p 1234 -o foo -f unknown").unwrap_err().kind,
clap::ErrorKind::InvalidValue);

// test out overriding these params by setting flags
assert_eq!(config.include_idle, false);
assert_eq!(config.gil_only, false);
assert_eq!(config.include_thread_ids, false);

let config_flags = Config::from_args(&split("py-spy r -p 1234 -o foo --idle --gil --threads")).unwrap();
let config_flags = get_config("py-spy r -p 1234 -o foo --idle --gil --threads").unwrap();
assert_eq!(config_flags.include_idle, true);
assert_eq!(config_flags.gil_only, true);
assert_eq!(config_flags.include_thread_ids, true);
Expand All @@ -298,34 +315,34 @@ mod tests {
#[test]
fn test_parse_dump_args() {
// basic use case
let config = Config::from_args(&split("py-spy dump --pid 1234")).unwrap();
let config = get_config("py-spy dump --pid 1234").unwrap();
assert_eq!(config.pid, Some(1234));
assert_eq!(config.command, String::from("dump"));

// short version
let short_config = Config::from_args(&split("py-spy d -p 1234")).unwrap();
let short_config = get_config("py-spy d -p 1234").unwrap();
assert_eq!(config, short_config);

// missing the --pid argument should fail
assert_eq!(Config::from_args(&split("py-spy dump")).unwrap_err().kind,
assert_eq!(get_config("py-spy dump").unwrap_err().kind,
clap::ErrorKind::MissingRequiredArgument);
}

#[test]
fn test_parse_top_args() {
// basic use case
let config = Config::from_args(&split("py-spy top --pid 1234")).unwrap();
let config = get_config("py-spy top --pid 1234").unwrap();
assert_eq!(config.pid, Some(1234));
assert_eq!(config.command, String::from("top"));

// short version
let short_config = Config::from_args(&split("py-spy t -p 1234")).unwrap();
let short_config = get_config("py-spy t -p 1234").unwrap();
assert_eq!(config, short_config);
}

#[test]
fn test_parse_args() {
assert_eq!(Config::from_args(&split("py-spy dude")).unwrap_err().kind,
assert_eq!(get_config("py-spy dude").unwrap_err().kind,
clap::ErrorKind::UnrecognizedSubcommand);
}
}
2 changes: 1 addition & 1 deletion src/python_spy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ mod tests {
assert!(!is_python_lib("/lib/heapq.cpython-36m-darwin.dylib"));
}

#[cfg(target_os="linux")]
#[cfg(any(target_os="linux", target_os="freebsd"))]
#[test]
fn test_is_python_lib() {
// libpython bundled by pyinstaller https://github.com/benfred/py-spy/issues/42
Expand Down

0 comments on commit 4a490a2

Please sign in to comment.