fix(windows): use a well-known path for the crash handler socket (#3444)

I didn't notice that the socket is a Unix domain socket, and not a named
pipe, so it shows up in the normal Windows filesystem.

Since I'm trying to get rid of the `set_current_dir` call at startup,
this needs to use a well-known path instead of a relative path.
(https://github.com/firezone/firezone/pull/3430/files#diff-8ee58783aeb973dcbf764b93d3038dd0133d981cc0caae8c5429020eb002a52eL62)

So I stuck it in `%LOCALAPPDATA%/data/`.


![image](https://github.com/firezone/firezone/assets/13400041/85335b3f-064f-4c8d-be50-3f4e98b9302c)

I manually tested and made sure that the crash dump is written when we
pass `--crash-on-purpose`, so the client and server are able to reach
each other correctly.

---------

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
This commit is contained in:
Reactor Scram
2024-01-30 15:28:18 -06:00
committed by GitHub
parent 9096eee396
commit f2f8464f02
2 changed files with 16 additions and 8 deletions

View File

@@ -1,6 +1,6 @@
use anyhow::Result;
use clap::{Args, Parser};
use std::{os::windows::process::CommandExt, process::Command};
use std::{os::windows::process::CommandExt, path::PathBuf, process::Command};
mod about;
mod auth;
@@ -107,7 +107,7 @@ pub(crate) fn run() -> Result<()> {
Ok(())
}
}
Some(Cmd::CrashHandlerServer) => crash_handling::server(),
Some(Cmd::CrashHandlerServer { socket_path }) => crash_handling::server(socket_path),
Some(Cmd::Debug { command }) => debug_commands::run(command),
// If we already tried to elevate ourselves, don't try again
Some(Cmd::Elevated) => gui::run(GuiParams {
@@ -136,7 +136,9 @@ struct Cli {
#[derive(clap::Subcommand)]
pub enum Cmd {
CrashHandlerServer,
CrashHandlerServer {
socket_path: PathBuf,
},
Debug {
#[command(subcommand)]
command: debug_commands::Cmd,

View File

@@ -9,8 +9,6 @@ use anyhow::{anyhow, bail, Context, Result};
use crash_handler::CrashHandler;
use std::{fs::File, io::Write, path::PathBuf};
const SOCKET_NAME: &str = "dev.firezone.client.crash_handler";
/// Attaches a crash handler to the client process
///
/// Returns a CrashHandler that must be kept alive until the program exits.
@@ -46,8 +44,8 @@ pub(crate) fn attach_handler() -> Result<CrashHandler> {
///
/// <https://jake-shadle.github.io/crash-reporting/#implementation>
/// <https://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md#terminology>
pub(crate) fn server() -> Result<()> {
let mut server = minidumper::Server::with_name(SOCKET_NAME)?;
pub(crate) fn server(socket_path: PathBuf) -> Result<()> {
let mut server = minidumper::Server::with_name(&*socket_path)?;
let ab = std::sync::atomic::AtomicBool::new(false);
server.run(Box::new(Handler), &ab, None)?;
Ok(())
@@ -55,6 +53,13 @@ pub(crate) fn server() -> Result<()> {
fn start_server_and_connect() -> Result<(minidumper::Client, std::process::Child)> {
let exe = std::env::current_exe().context("unable to find our own exe path")?;
// Path of a Unix domain socket for IPC with the crash handler server
// <https://github.com/EmbarkStudios/crash-handling/issues/10>
let socket_path = app_local_data_dir()
.context("couldn't compute crash handler socket path")?
.join("data")
.join("crash_handler_pipe");
let mut server = None;
// I don't understand why there's a loop here. The original was an infinite loop,
@@ -63,7 +68,7 @@ fn start_server_and_connect() -> Result<(minidumper::Client, std::process::Child
for _ in 0..10 {
// Create the crash client first so we can error out if another instance of
// the Firezone client is already using this socket for crash handling.
if let Ok(client) = minidumper::Client::with_name(SOCKET_NAME) {
if let Ok(client) = minidumper::Client::with_name(&*socket_path) {
return Ok((
client,
server.ok_or_else(|| {
@@ -77,6 +82,7 @@ fn start_server_and_connect() -> Result<(minidumper::Client, std::process::Child
server = Some(
std::process::Command::new(&exe)
.arg("crash-handler-server")
.arg(&socket_path)
.spawn()
.context("unable to spawn server process")?,
);