Skip to content

Commit 1fd4bb9

Browse files
LeiyksLeiyks
committed
fix(spawn_worker): defer trampoline self-deletion via env var instead of stable path
Move the fix to trampoline.c: when DD_SPAWN_WORKER_STABLE_TRAMPOLINE is set, defer unlink(argv[1]) to after the entry function returns so Valgrind can read DWARF symbols. Remove the Rust stable-path implementation (OnceLock, atexit, stack-alloc path) — it was solving the problem at the wrong layer.
1 parent 1b8cb61 commit 1fd4bb9

2 files changed

Lines changed: 36 additions & 129 deletions

File tree

‎spawn_worker/src/trampoline.c‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,10 @@ struct trampoline_data {
3333

3434
int main(int argc, char *argv[]) {
3535
if (argc > 3) {
36-
// remove the temp file of this trampoline
37-
if (*argv[1]) {
36+
// Defer self-deletion when DD_SPAWN_WORKER_STABLE_TRAMPOLINE is set so
37+
// Valgrind can read DWARF symbols after execve; otherwise unlink immediately.
38+
bool stable = getenv("DD_SPAWN_WORKER_STABLE_TRAMPOLINE") != NULL;
39+
if (!stable && *argv[1]) {
3840
unlink(argv[1]);
3941
}
4042

@@ -200,6 +202,9 @@ int main(int argc, char *argv[]) {
200202

201203
(*fn)(&startup_data);
202204
#endif
205+
if (stable && *argv[1]) {
206+
unlink(argv[1]);
207+
}
203208
return 0;
204209
}
205210

‎spawn_worker/src/unix/spawn.rs‎

Lines changed: 29 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,6 @@ use std::fs::File;
8181
use std::ffi::CStr;
8282
use std::io;
8383
use std::ops::RangeInclusive;
84-
#[cfg(target_os = "linux")]
85-
use std::sync::OnceLock;
8684
use std::{
8785
env,
8886
ffi::{self, CString, OsString},
@@ -456,145 +454,49 @@ impl SpawnWorker {
456454
// if we're here then exec has failed
457455
let fexecve_error = std::io::Error::last_os_error();
458456

459-
// When DD_SPAWN_WORKER_STABLE_TRAMPOLINE is set, write the trampoline to
460-
// a stable per-process path so Valgrind can open it for debug symbols
461-
// before exec completes. Default (unset): keep the random-path behaviour.
462-
let stable_env_key = b"DD_SPAWN_WORKER_STABLE_TRAMPOLINE\0";
463-
let use_stable = {
464-
let val = libc::getenv(stable_env_key.as_ptr() as *const libc::c_char);
465-
!val.is_null()
457+
let mut temp_path = [0u8; 256];
458+
let tmpdir = libc::getenv("TMPDIR".as_ptr() as *const libc::c_char)
459+
as *const libc::c_char;
460+
let tmpdir = if tmpdir.is_null() {
461+
b"/tmp"
462+
} else {
463+
CStr::from_ptr(tmpdir).to_bytes()
466464
};
467-
468-
if use_stable {
469-
// Stack-allocated path /tmp/.dd-trampoline-<pid> — no heap post-fork.
470-
let mut path_buf = [0u8; 48];
471-
{
472-
let prefix = b"/tmp/.dd-trampoline-";
473-
path_buf[..prefix.len()].copy_from_slice(prefix);
474-
let mut n = libc::getpid() as u32;
475-
let mut digits = [0u8; 20];
476-
let mut nd = 0usize;
477-
loop {
478-
digits[nd] = b'0' + (n % 10) as u8;
479-
nd += 1;
480-
n /= 10;
481-
if n == 0 {
482-
break;
483-
}
484-
}
485-
digits[..nd].reverse();
486-
path_buf[prefix.len()..prefix.len() + nd]
487-
.copy_from_slice(&digits[..nd]);
488-
}
489-
let path_ptr = path_buf.as_ptr() as *const libc::c_char;
490-
491-
// Fork-safe atexit: recomputes path from getpid() so each process
492-
// cleans up its own file.
493-
extern "C" fn cleanup_trampoline() {
494-
unsafe {
495-
let mut buf = [0u8; 48];
496-
let prefix = b"/tmp/.dd-trampoline-";
497-
buf[..prefix.len()].copy_from_slice(prefix);
498-
let mut n = libc::getpid() as u32;
499-
let mut digits = [0u8; 20];
500-
let mut nd = 0usize;
501-
loop {
502-
digits[nd] = b'0' + (n % 10) as u8;
503-
nd += 1;
504-
n /= 10;
505-
if n == 0 {
506-
break;
507-
}
508-
}
509-
digits[..nd].reverse();
510-
buf[prefix.len()..prefix.len() + nd].copy_from_slice(&digits[..nd]);
511-
libc::unlink(buf.as_ptr() as *const libc::c_char);
512-
}
465+
if tmpdir.len() < 220 {
466+
temp_path[..tmpdir.len()].copy_from_slice(tmpdir);
467+
let mut off = tmpdir.len();
468+
let spawn_prefix = b"/dd-ipc-spawn_";
469+
temp_path[off..off + spawn_prefix.len()].copy_from_slice(spawn_prefix);
470+
off += spawn_prefix.len();
471+
for _ in 0..8 {
472+
temp_path[off] = fastrand::alphanumeric() as u8;
473+
off += 1;
513474
}
514475

515-
// Presence-only lock — after fork, the child inherits the state but
516-
// cleanup_trampoline recomputes the path via getpid().
517-
static ATEXIT_REGISTERED: OnceLock<()> = OnceLock::new();
518-
476+
let path = Vec::from_raw_parts(temp_path.as_mut_ptr(), off, off);
477+
let path = CString::from_vec_with_nul_unchecked(path);
478+
let path_ptr = path.as_ptr();
519479
let tmpfd = libc::open(
520480
path_ptr,
521-
libc::O_CREAT | libc::O_EXCL | libc::O_RDWR,
481+
libc::O_CREAT | libc::O_RDWR,
522482
libc::S_IRWXU as libc::c_uint,
523483
);
524-
if tmpfd >= 0 {
525-
let written = libc::sendfile(
484+
if tmpfd < 0 {
485+
// We'll leak it, executing Drop of path is forbidden.
486+
std::mem::forget(path);
487+
} else {
488+
libc::sendfile(
526489
tmpfd,
527490
fd.as_raw_fd(),
528491
std::ptr::null_mut(),
529492
crate::TRAMPOLINE_BIN.len(),
530493
);
531494
libc::close(tmpfd);
532-
if written < 0 || written as usize != crate::TRAMPOLINE_BIN.len() {
533-
// Partial / failed write — remove corrupt file and fall
534-
// through to the panic below.
535-
libc::unlink(path_ptr);
536-
} else {
537-
ATEXIT_REGISTERED.get_or_init(|| {
538-
libc::atexit(cleanup_trampoline);
539-
});
540-
// argv[1] is "" — trampoline skips unlink when *argv[1] == '\0'.
541-
libc::execve(path_ptr, argv.as_ptr(), envp.as_ptr());
542-
// execve failed — remove file so next attempt recreates it.
543-
libc::unlink(path_ptr);
544-
}
545-
} else {
546-
// O_EXCL failed: same-PID orphan (killed before atexit). Reuse it.
547-
ATEXIT_REGISTERED.get_or_init(|| {
548-
libc::atexit(cleanup_trampoline);
549-
});
495+
argv.set(1, path);
496+
550497
libc::execve(path_ptr, argv.as_ptr(), envp.as_ptr());
551-
libc::unlink(path_ptr);
552-
}
553-
} else {
554-
let mut temp_path = [0u8; 256];
555-
let tmpdir = libc::getenv("TMPDIR".as_ptr() as *const libc::c_char)
556-
as *const libc::c_char;
557-
let tmpdir = if tmpdir.is_null() {
558-
b"/tmp"
559-
} else {
560-
CStr::from_ptr(tmpdir).to_bytes()
561-
};
562-
if tmpdir.len() < 220 {
563-
temp_path[..tmpdir.len()].copy_from_slice(tmpdir);
564-
let mut off = tmpdir.len();
565-
let spawn_prefix = b"/dd-ipc-spawn_";
566-
temp_path[off..off + spawn_prefix.len()].copy_from_slice(spawn_prefix);
567-
off += spawn_prefix.len();
568-
for _ in 0..8 {
569-
temp_path[off] = fastrand::alphanumeric() as u8;
570-
off += 1;
571-
}
572-
573-
let path = Vec::from_raw_parts(temp_path.as_mut_ptr(), off, off);
574-
let path = CString::from_vec_with_nul_unchecked(path);
575-
let path_ptr = path.as_ptr();
576-
let tmpfd = libc::open(
577-
path_ptr,
578-
libc::O_CREAT | libc::O_RDWR,
579-
libc::S_IRWXU as libc::c_uint,
580-
);
581-
if tmpfd < 0 {
582-
// We'll leak it, executing Drop of path is forbidden.
583-
std::mem::forget(path);
584-
} else {
585-
libc::sendfile(
586-
tmpfd,
587-
fd.as_raw_fd(),
588-
std::ptr::null_mut(),
589-
crate::TRAMPOLINE_BIN.len(),
590-
);
591-
libc::close(tmpfd);
592-
argv.set(1, path);
593-
594-
libc::execve(path_ptr, argv.as_ptr(), envp.as_ptr());
595-
596-
libc::unlink(temp_path.as_ptr() as *const libc::c_char);
597-
}
498+
499+
libc::unlink(temp_path.as_ptr() as *const libc::c_char);
598500
}
599501
}
600502

0 commit comments

Comments
 (0)