diff --git a/CHANGELOG.md b/CHANGELOG.md index 918b0c7d84..d802877fba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ **Fixes**: +- Native: store daemon logs, minidumps, crash envelopes, and scratch files in `.run` directories so they are cleaned up with the run instead of accumulating in the database root. Minidumps can still be retained with `cache_keep`, which stores `.dmp` sidecars alongside cached envelopes. ([#1976](https://github.com/getsentry/sentry-native/pull/1976)) - Linux/ARM32: prevent recursive crashes when libunwind receives an unmapped initial instruction pointer during crash handling. ([#1977](https://github.com/getsentry/sentry-native/pull/1977)) ## 0.16.3 diff --git a/src/backends/native/minidump/sentry_minidump_macos.c b/src/backends/native/minidump/sentry_minidump_macos.c index 982024ddde..f1456f5f6a 100644 --- a/src/backends/native/minidump/sentry_minidump_macos.c +++ b/src/backends/native/minidump/sentry_minidump_macos.c @@ -1314,14 +1314,14 @@ write_module_headers_from_capture(minidump_writer_t *writer, { const size_t HEADER_PAGE_SIZE = 4096; - // Build path: {database_path}/__sentry-modheaders - const char *db_path = writer->crash_ctx->database_path; - size_t db_len = strlen(db_path); + // Build path: {run_path}/__sentry-modheaders + const char *run_path = writer->crash_ctx->run_path; + size_t run_len = strlen(run_path); char hdr_path[SENTRY_CRASH_MAX_PATH]; - if (db_len + 22 >= sizeof(hdr_path)) { + if (run_len + 22 >= sizeof(hdr_path)) { return 0; } - snprintf(hdr_path, sizeof(hdr_path), "%s/__sentry-modheaders", db_path); + snprintf(hdr_path, sizeof(hdr_path), "%s/__sentry-modheaders", run_path); int fd = open(hdr_path, O_RDONLY); if (fd < 0) { @@ -1672,12 +1672,12 @@ write_memory_list_stream(minidump_writer_t *writer, minidump_directory_t *dir) // Clean up the capture file written by the signal handler since // we used VM regions instead. - const char *db_path = writer->crash_ctx->database_path; - size_t db_len = strlen(db_path); + const char *run_path = writer->crash_ctx->run_path; + size_t run_len = strlen(run_path); char hdr_path[SENTRY_CRASH_MAX_PATH]; - if (db_len + 22 < sizeof(hdr_path)) { + if (run_len + 22 < sizeof(hdr_path)) { snprintf( - hdr_path, sizeof(hdr_path), "%s/__sentry-modheaders", db_path); + hdr_path, sizeof(hdr_path), "%s/__sentry-modheaders", run_path); unlink(hdr_path); } diff --git a/src/backends/native/sentry_crash_context.h b/src/backends/native/sentry_crash_context.h index 20681b812b..5a51c06ad1 100644 --- a/src/backends/native/sentry_crash_context.h +++ b/src/backends/native/sentry_crash_context.h @@ -312,8 +312,8 @@ typedef struct { #endif // Sentry-specific metadata paths - char database_path[SENTRY_CRASH_MAX_PATH]; // Database directory for all - // files + char database_path[SENTRY_CRASH_MAX_PATH]; // Shared across runs + char run_path[SENTRY_CRASH_MAX_PATH]; // For current run char event_path[SENTRY_CRASH_MAX_PATH]; char breadcrumb1_path[SENTRY_CRASH_MAX_PATH]; char breadcrumb2_path[SENTRY_CRASH_MAX_PATH]; diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index 6fa65ecd79..7522074174 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -4003,14 +4003,13 @@ sentry__process_crash(const sentry_options_t *options, sentry_crash_ipc_t *ipc) bool use_native_mode = (mode == SENTRY_CRASH_REPORTING_MODE_NATIVE || mode == SENTRY_CRASH_REPORTING_MODE_NATIVE_WITH_MINIDUMP); - // Generate minidump path in database directory + // Generate minidump path in run directory char minidump_path[SENTRY_CRASH_MAX_PATH] = { 0 }; - const char *db_dir = ctx->database_path; + const char *run_dir = ctx->run_path; if (need_minidump) { int path_len = snprintf(minidump_path, sizeof(minidump_path), - "%s/sentry-minidump-%lu-%lu.dmp", db_dir, - (unsigned long)ctx->crashed_pid, (unsigned long)ctx->crashed_tid); + "%s/__sentry-crash.dmp", run_dir); if (path_len < 0 || path_len >= (int)sizeof(minidump_path)) { SENTRY_WARN("Minidump path truncated or invalid"); @@ -4082,29 +4081,19 @@ sentry__process_crash(const sentry_options_t *options, sentry_crash_ipc_t *ipc) sentry_path_t *ev_path = sentry__path_from_str(event_path); sentry_path_t *run_folder = ev_path ? sentry__path_dir(ev_path) : NULL; - // Acquire the run directory lock file so that process_old_runs() in a - // new SDK run will skip this directory while the daemon is still - // processing the crash. The crashed process's flock() is released on - // death, so without this the new run could delete the directory. - sentry_filelock_t *run_lock = NULL; - if (run_folder) { - sentry_path_t *lock_path = sentry__path_append_str(run_folder, ".lock"); - if (lock_path) { - run_lock = sentry__filelock_new(lock_path); - if (run_lock) { - if (!sentry__filelock_try_lock(run_lock)) { - SENTRY_WARN("daemon could not acquire run folder lock"); - sentry__filelock_free(run_lock); - run_lock = NULL; - } - } - } + // The crashing process dumps its pending logs, sessions, and transactions + // before notifying the daemon. Queue those before writing the crash + // envelope so an attachment-ref prewrite is not captured a second time. + if (run_folder && options && options->transport && options->run) { + sentry__process_run_envelopes(options, run_folder); + } else { + SENTRY_DEBUG("No run folder or transport for additional envelopes"); } - // Create envelope file in database directory + // Create envelope file in run directory char envelope_path[SENTRY_CRASH_MAX_PATH]; - int path_len = snprintf(envelope_path, sizeof(envelope_path), - "%s/sentry-envelope-%lu.env", db_dir, (unsigned long)ctx->crashed_pid); + int path_len = snprintf( + envelope_path, sizeof(envelope_path), "%s", ctx->envelope_path); if (path_len < 0 || path_len >= (int)sizeof(envelope_path)) { SENTRY_WARN("Envelope path truncated or invalid"); @@ -4112,10 +4101,6 @@ sentry__process_crash(const sentry_options_t *options, sentry_crash_ipc_t *ipc) if (run_folder) { sentry__path_free(run_folder); } - if (run_lock) { - sentry__filelock_unlock(run_lock); - sentry__filelock_free(run_lock); - } goto done; } @@ -4216,10 +4201,6 @@ sentry__process_crash(const sentry_options_t *options, sentry_crash_ipc_t *ipc) if (run_folder) { sentry__path_free(run_folder); } - if (run_lock) { - sentry__filelock_unlock(run_lock); - sentry__filelock_free(run_lock); - } goto done; } SENTRY_DEBUG("Envelope written successfully"); @@ -4335,57 +4316,9 @@ sentry__process_crash(const sentry_options_t *options, sentry_crash_ipc_t *ipc) sentry_value_decref(crash_event); } - // Send all other envelopes from run folder (logs, etc.) before cleanup - if (run_folder && options && options->transport && options->run) { - SENTRY_DEBUG("Checking for additional envelopes in run folder"); - sentry_pathiter_t *piter = sentry__path_iter_directory(run_folder); - if (piter) { - SENTRY_DEBUG("Iterating run folder for envelope files"); - const sentry_path_t *file_path; - int envelope_count = 0; - while ((file_path = sentry__pathiter_next(piter)) != NULL) { - // Check if this is an envelope file (ends with .envelope) - const char *path_str = file_path->path; - size_t len = strlen(path_str); - if (len > 9 && strcmp(path_str + len - 9, ".envelope") == 0) { - SENTRY_DEBUGF( - "Sending envelope from run folder: %s", path_str); - sentry_envelope_t *run_envelope - = sentry__envelope_from_path(file_path); - if (run_envelope) { - sentry__capture_envelope( - options->transport, run_envelope, options); - envelope_count++; - } else { - SENTRY_WARNF("Failed to load envelope: %s", path_str); - } - } - } - SENTRY_DEBUGF( - "Sent %d additional envelopes from run folder", envelope_count); - sentry__pathiter_free(piter); - } else { - SENTRY_DEBUG("Could not iterate run folder"); - } - } else { - SENTRY_DEBUG("No run folder or transport for additional envelopes"); - } - - // Clean up the entire run folder (contains breadcrumbs, etc.) - if (run_folder) { - SENTRY_DEBUG("Cleaning up run folder"); - sentry__path_remove_all(run_folder); - sentry__path_free(run_folder); - } + sentry__path_free(run_folder); sentry__path_free(ev_path); - // Release and clean up the lock file - if (run_lock) { - sentry__filelock_unlock(run_lock); - sentry__filelock_free(run_lock); - } - SENTRY_DEBUG("Cleaned up crash run folder and lock file"); - SENTRY_DEBUG("Crash processing completed successfully"); done: @@ -4394,6 +4327,20 @@ sentry__process_crash(const sentry_options_t *options, sentry_crash_ipc_t *ipc) return crash_captured; } +static void +remove_pending_run_envelopes(const sentry_path_t *run_path) +{ + sentry_pathiter_t *it = sentry__path_iter_directory(run_path); + const sentry_path_t *file; + while (it && (file = sentry__pathiter_next(it)) != NULL) { + if (sentry__path_is_file(file) && !sentry__path_is_symlink(file) + && sentry__path_ends_with(file, ".envelope")) { + sentry__path_remove(file); + } + } + sentry__pathiter_free(it); +} + /** * Check if parent process is still alive */ @@ -4472,17 +4419,14 @@ sentry__crash_daemon_main(pid_t app_pid, uint64_t app_tid, HANDLE event_handle, } // Set up logging to file for daemon BEFORE redirecting streams - // Use same naming scheme as shared memory (PID ^ TID hash) to handle - // multiple threads in same process char log_path[SENTRY_CRASH_MAX_PATH]; FILE *log_file = NULL; - uint32_t id = (uint32_t)((app_pid ^ (app_tid & 0xFFFFFFFF)) & 0xFFFFFFFF); #if defined(SENTRY_PLATFORM_WINDOWS) // On Windows, convert UTF-8 path to wide characters for proper file // handling int log_path_len = snprintf(log_path, sizeof(log_path), - "%s\\sentry-daemon-%08x.log", ipc->shmem->database_path, id); + "%s\\sentry-daemon.log", ipc->shmem->run_path); if (log_path_len > 0 && log_path_len < (int)sizeof(log_path)) { wchar_t *wlog_path = sentry__string_to_wstr(log_path); @@ -4493,7 +4437,7 @@ sentry__crash_daemon_main(pid_t app_pid, uint64_t app_tid, HANDLE event_handle, } #else int log_path_len = snprintf(log_path, sizeof(log_path), - "%s/sentry-daemon-%08x.log", ipc->shmem->database_path, id); + "%s/sentry-daemon.log", ipc->shmem->run_path); if (log_path_len > 0 && log_path_len < (int)sizeof(log_path)) { log_file = fopen(log_path, "w"); @@ -4556,6 +4500,8 @@ sentry__crash_daemon_main(pid_t app_pid, uint64_t app_tid, HANDLE event_handle, return 1; } + sentry_options_set_database_path(options, ipc->shmem->database_path); + // Use debug logging and screenshot settings from parent process sentry_options_set_debug(options, ipc->shmem->debug_enabled); options->attach_screenshot = ipc->shmem->attach_screenshot; @@ -4597,17 +4543,17 @@ sentry__crash_daemon_main(pid_t app_pid, uint64_t app_tid, HANDLE event_handle, options->user_agent = sentry__string_clone(ipc->shmem->user_agent); } - // Create run with database path - SENTRY_DEBUG("Creating run with database path"); - sentry_path_t *db_path = sentry__path_from_str(ipc->shmem->database_path); - if (db_path) { - options->run = sentry__run_new(db_path); + // Adopt existing run + SENTRY_DEBUG("Adopting existing run"); + sentry_path_t *run_path = sentry__path_from_str(ipc->shmem->run_path); + if (options->database_path && run_path) { + options->run = sentry__run_adopt(options->database_path, run_path); if (options->run) { options->run->require_user_consent = ipc->shmem->require_user_consent; } - sentry__path_free(db_path); } + sentry__path_free(run_path); // Set external crash reporter if configured if (ipc->shmem->external_reporter_path[0] != '\0') { @@ -4723,13 +4669,15 @@ sentry__crash_daemon_main(pid_t app_pid, uint64_t app_tid, HANDLE event_handle, if (rv != 0) { SENTRY_WARN("transport did not shut down cleanly"); } - dumped_envelopes = sentry__transport_dump_queue( - options->transport, options->run); - if (rv == 0 && !dumped_envelopes && options->run) { - sentry__run_clean(options->run, true); + + if (crash_processed) { + dumped_envelopes = sentry__transport_dump_queue( + options->transport, options->run); + if (rv == 0 && !dumped_envelopes && options->run) { + remove_pending_run_envelopes(options->run->run_path); + } } } - sentry_options_free(options); } if (crash_processed) { // Mark as done @@ -4743,9 +4691,14 @@ sentry__crash_daemon_main(pid_t app_pid, uint64_t app_tid, HANDLE event_handle, // Close log file if (log_file) { + sentry__logger_disable(); fclose(log_file); } + if (options) { + sentry_options_free(options); + } + return 0; } diff --git a/src/backends/native/sentry_crash_handler.c b/src/backends/native/sentry_crash_handler.c index 77fd119751..887aa97681 100644 --- a/src/backends/native/sentry_crash_handler.c +++ b/src/backends/native/sentry_crash_handler.c @@ -160,6 +160,8 @@ get_tid(void) # endif } +// safe_strxxx are only used on macOS (for stack path and module names) +# if defined(SENTRY_PLATFORM_MACOS) /** * Safe string length (signal-safe) */ @@ -173,8 +175,6 @@ safe_strlen(const char *s) return len; } -// safe_strncpy is only used on macOS (for stack path and module names) -# if defined(SENTRY_PLATFORM_MACOS) /** * Safe string copy (signal-safe) */ @@ -237,26 +237,26 @@ safe_uint_to_str(char *buf, size_t buf_size, unsigned int value) } /** - * Build stack path signal-safely: "{database_path}/__sentry-stack{index}" + * Build stack path signal-safely: "{run_path}/__sentry-stack{index}" * Returns total length or 0 on error/truncation */ static size_t safe_build_stack_path( - char *dest, size_t dest_size, const char *database_path, unsigned int index) + char *dest, size_t dest_size, const char *run_path, unsigned int index) { if (!dest || dest_size == 0) { return 0; } - // Copy database path + // Copy run path size_t pos = 0; - size_t db_len = safe_strlen(database_path); - if (db_len >= dest_size) { + size_t run_len = safe_strlen(run_path); + if (run_len >= dest_size) { dest[0] = '\0'; return 0; // Would truncate } - safe_strncpy(dest, database_path, dest_size); - pos = db_len; + safe_strncpy(dest, run_path, dest_size); + pos = run_len; // Append "/__sentry-stack" const char *suffix = "/__sentry-stack"; @@ -568,11 +568,11 @@ crash_signal_handler(int signum, siginfo_t *info, void *context) } if (actual_stack_size > 0) { - // Create stack file path in database directory + // Create stack file path in run directory // (signal-safe) char stack_path[SENTRY_CRASH_MAX_PATH]; size_t len = safe_build_stack_path( - stack_path, sizeof(stack_path), ctx->database_path, i); + stack_path, sizeof(stack_path), ctx->run_path, i); // Check for failure/truncation if (len == 0) { @@ -700,9 +700,9 @@ crash_signal_handler(int signum, siginfo_t *info, void *context) // File format: module[0] header (4096 bytes) || module[1] header || ... { char hdr_path[SENTRY_CRASH_MAX_PATH]; - size_t pos = safe_strlen(ctx->database_path); + size_t pos = safe_strlen(ctx->run_path); if (pos + 22 < sizeof(hdr_path)) { // "/__sentry-modheaders\0" - safe_strncpy(hdr_path, ctx->database_path, sizeof(hdr_path)); + safe_strncpy(hdr_path, ctx->run_path, sizeof(hdr_path)); const char *suffix = "/__sentry-modheaders"; for (size_t si = 0; suffix[si] != '\0'; si++) { hdr_path[pos++] = suffix[si]; @@ -785,74 +785,41 @@ crash_signal_handler(int signum, siginfo_t *info, void *context) // Dump daemon log for debugging (uses stdio, safe after page allocator // enabled) - // Extract the shm identifier for log path construction - // macOS: shm_path = "{tmpdir}/.sentry-shm-{id}", Linux: shm_name = - // "/s-{id}" -# if defined(SENTRY_PLATFORM_MACOS) - const char *shm_id_src = ipc ? ipc->shm_path : ""; -# else - const char *shm_id_src = ipc ? ipc->shm_name : ""; -# endif - if (shm_id_src[0] != '\0' && ctx && ctx->database_path[0] != '\0') { - // Extract hex ID after last '-' in shm name/path - const char *shm_id = NULL; - for (const char *p = shm_id_src; *p; p++) { - if (*p == '-') { - shm_id = p + 1; - } + if (ctx && ctx->run_path[0] != '\0') { + char log_path[SENTRY_CRASH_MAX_PATH]; + const char suffix[] = "/sentry-daemon.log"; + int len = 0; + // Manually build path string (signal-safe) + for (const char *p = ctx->run_path; + *p && len < (int)(sizeof(log_path) - sizeof(suffix)); p++) { + log_path[len++] = *p; } - - if (shm_id) { - char log_path[SENTRY_CRASH_MAX_PATH]; - int len = 0; - // Manually build path string (signal-safe) - for (const char *p = ctx->database_path; - *p && len < (int)sizeof(log_path) - 30; p++) { - log_path[len++] = *p; - } - const char *suffix = "/sentry-daemon-"; - for (const char *p = suffix; *p && len < (int)sizeof(log_path) - 15; - p++) { - log_path[len++] = *p; - } - for (const char *p = shm_id; *p && len < (int)sizeof(log_path) - 5; - p++) { - log_path[len++] = *p; - } - const char *ext = ".log"; - for (const char *p = ext; *p && len < (int)sizeof(log_path) - 1; - p++) { - log_path[len++] = *p; - } - log_path[len] = '\0'; - - // Try to open and dump log file - int fd = open(log_path, O_RDONLY); - if (fd >= 0) { - // Use sizeof()-1 for string literals (signal-safe) - ssize_t rv = write(STDERR_FILENO, "\n========== Daemon Log (", - sizeof("\n========== Daemon Log (") - 1); - (void)rv; // Ignore write errors in signal handler - rv = write(STDERR_FILENO, shm_id, safe_strlen(shm_id)); - (void)rv; - rv = write(STDERR_FILENO, ") ==========\n", - sizeof(") ==========\n") - 1); - (void)rv; - - char buf[1024]; - ssize_t n; - while ((n = read(fd, buf, sizeof(buf))) > 0) { - rv = write(STDERR_FILENO, buf, n); - (void)rv; - } - - rv = write(STDERR_FILENO, - "=========================================\n\n", - sizeof("=========================================\n\n") - - 1); + for (const char *p = suffix; *p && len < (int)sizeof(log_path) - 1; + p++) { + log_path[len++] = *p; + } + log_path[len] = '\0'; + + // Try to open and dump log file + int fd = open(log_path, O_RDONLY); + if (fd >= 0) { + // Use sizeof()-1 for string literals (signal-safe) + ssize_t rv + = write(STDERR_FILENO, "\n========== Daemon Log ==========\n", + sizeof("\n========== Daemon Log ==========\n") - 1); + (void)rv; // Ignore write errors in signal handler + + char buf[1024]; + ssize_t n; + while ((n = read(fd, buf, sizeof(buf))) > 0) { + rv = write(STDERR_FILENO, buf, n); (void)rv; - close(fd); } + + rv = write(STDERR_FILENO, "================================\n\n", + sizeof("================================\n\n") - 1); + (void)rv; + close(fd); } } diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index ffc14c2182..20f7091b9c 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -321,6 +321,13 @@ native_backend_startup( sentry_path_t *run_path = options->run->run_path; sentry_path_t *db_path = options->database_path; +#ifdef _WIN32 + strncpy_s(ctx->run_path, sizeof(ctx->run_path), run_path->path, _TRUNCATE); +#else + strncpy(ctx->run_path, run_path->path, sizeof(ctx->run_path) - 1); + ctx->run_path[sizeof(ctx->run_path) - 1] = '\0'; +#endif + // Store database path for daemon use if (db_path) { #ifdef _WIN32 @@ -585,12 +592,17 @@ native_backend_shutdown(sentry_backend_t *backend) // handler on iOS) sentry__crash_handler_shutdown(); + bool daemon_stopped = false; #if defined(SENTRY_PLATFORM_UNIX) && !defined(SENTRY_PLATFORM_IOS) // Terminate daemon (Unix) if (state->daemon_pid > 0) { kill(state->daemon_pid, SIGTERM); // Wait for daemon to exit - waitpid(state->daemon_pid, NULL, 0); + pid_t wait_result; + do { + wait_result = waitpid(state->daemon_pid, NULL, 0); + } while (wait_result < 0 && errno == EINTR); + daemon_stopped = wait_result == state->daemon_pid; } #elif defined(SENTRY_PLATFORM_WINDOWS) // Terminate daemon (Windows) @@ -600,12 +612,27 @@ native_backend_shutdown(sentry_backend_t *backend) if (hDaemon) { TerminateProcess(hDaemon, 0); // Wait for daemon to exit (with timeout) - WaitForSingleObject(hDaemon, 5000); // 5 second timeout + daemon_stopped + = WaitForSingleObject(hDaemon, 5000) == WAIT_OBJECT_0; CloseHandle(hDaemon); } } #endif + if (daemon_stopped && state->ipc && state->ipc->shmem + && state->ipc->shmem->run_path[0]) { + sentry_path_t *run_path + = sentry__path_from_str(state->ipc->shmem->run_path); + sentry_path_t *lock_path = run_path + ? sentry__path_append_str(run_path, ".daemon.lock") + : NULL; + if (lock_path) { + sentry__path_remove(lock_path); + } + sentry__path_free(lock_path); + sentry__path_free(run_path); + } + // Dump daemon log file for debugging (especially useful in CI). // This bypasses the SDK logger and writes straight to stderr, so it must // only run when debug logging was enabled. When debug is off the daemon @@ -614,69 +641,32 @@ native_backend_shutdown(sentry_backend_t *backend) if (state->ipc && state->ipc->shmem && state->ipc->shmem->debug_enabled) { char log_path[SENTRY_CRASH_MAX_PATH]; int log_path_len = -1; + FILE *log_file = NULL; - // Extract the unique ID from the shm name/path to find the daemon log - // Platform-specific: shm_name on Linux/Windows, shm_path on macOS #if defined(SENTRY_PLATFORM_WINDOWS) - const wchar_t *shm_id_w = wcsrchr(state->ipc->shm_name, L'-'); - if (shm_id_w) { - shm_id_w++; // Skip the '-' - char *shm_id = sentry__string_from_wstr(shm_id_w); - if (shm_id) { - log_path_len = _snprintf(log_path, sizeof(log_path), - "%s\\sentry-daemon-%s.log", - state->ipc->shmem->database_path, shm_id); - if (log_path_len > 0 && log_path_len < (int)sizeof(log_path)) { - wchar_t *wpath = sentry__string_to_wstr(log_path); - FILE *log_file = wpath ? _wfopen(wpath, L"r") : NULL; - sentry_free(wpath); - if (log_file) { - fprintf(stderr, - "\n========== Daemon Log (%s) ==========\n", - shm_id); - char line[1024]; - while (fgets(line, sizeof(line), log_file)) { - fprintf(stderr, "%s", line); - } - fprintf(stderr, - "=========================================\n\n"); - fclose(log_file); - } - } - sentry_free(shm_id); - } + log_path_len = _snprintf(log_path, sizeof(log_path), + "%s\\sentry-daemon.log", state->ipc->shmem->run_path); + if (log_path_len > 0 && log_path_len < (int)sizeof(log_path)) { + wchar_t *wpath = sentry__string_to_wstr(log_path); + log_file = wpath ? _wfopen(wpath, L"r") : NULL; + sentry_free(wpath); } #else - // On macOS: shm_path = "{tmpdir}/.sentry-shm-{id}" - // On Linux: shm_name = "/s-{id}" - // In both cases, the ID follows the last '-' -# if defined(SENTRY_PLATFORM_MACOS) - const char *shm_id_src = state->ipc->shm_path; -# else - const char *shm_id_src = state->ipc->shm_name; -# endif - const char *shm_id = shm_id_src[0] ? strrchr(shm_id_src, '-') : NULL; - if (shm_id) { - shm_id++; // Skip the '-' - log_path_len = snprintf(log_path, sizeof(log_path), - "%s/sentry-daemon-%s.log", state->ipc->shmem->database_path, - shm_id); - if (log_path_len > 0 && log_path_len < (int)sizeof(log_path)) { - FILE *log_file = fopen(log_path, "r"); - if (log_file) { - fprintf(stderr, "\n========== Daemon Log (%s) ==========\n", - shm_id); - char line[1024]; - while (fgets(line, sizeof(line), log_file)) { - fprintf(stderr, "%s", line); - } - fprintf(stderr, - "=========================================\n\n"); - fclose(log_file); - } - } + log_path_len = snprintf(log_path, sizeof(log_path), + "%s/sentry-daemon.log", state->ipc->shmem->run_path); + if (log_path_len > 0 && log_path_len < (int)sizeof(log_path)) { + log_file = fopen(log_path, "r"); } #endif + if (log_file) { + fprintf(stderr, "\n========== Daemon Log ==========\n"); + char line[1024]; + while (fgets(line, sizeof(line), log_file)) { + fprintf(stderr, "%s", line); + } + fprintf(stderr, "================================\n\n"); + fclose(log_file); + } } // Cleanup IPC diff --git a/src/sentry_database.c b/src/sentry_database.c index 55ef63d4d5..ea145de1f6 100644 --- a/src/sentry_database.c +++ b/src/sentry_database.c @@ -16,25 +16,13 @@ #include #include -sentry_run_t * -sentry__run_new(const sentry_path_t *database_path) +static sentry_run_t * +run_new(const sentry_path_t *database_path, sentry_path_t *run_path, + sentry_path_t *lock_path) { - sentry_uuid_t uuid = sentry_uuid_new_v4(); - char run_name[46]; - sentry_uuid_as_string(&uuid, run_name); - - // `/.run` - strcpy(&run_name[36], ".run"); - sentry_path_t *run_path = sentry__path_join_str(database_path, run_name); - if (!run_path) { - return NULL; - } - - // `/.run.lock` - strcpy(&run_name[40], ".lock"); - sentry_path_t *lock_path = sentry__path_join_str(database_path, run_name); - if (!lock_path) { + if (!database_path || !run_path || !lock_path) { sentry__path_free(run_path); + sentry__path_free(lock_path); return NULL; } @@ -80,7 +68,6 @@ sentry__run_new(const sentry_path_t *database_path) run->refcount = 1; run->require_user_consent = 0; run->user_consent = SENTRY_USER_CONSENT_UNKNOWN; - run->uuid = uuid; run->run_path = run_path; run->session_path = session_path; run->external_path = external_path; @@ -102,6 +89,41 @@ sentry__run_new(const sentry_path_t *database_path) return NULL; } +sentry_run_t * +sentry__run_new(const sentry_path_t *database_path) +{ + sentry_uuid_t uuid = sentry_uuid_new_v4(); + char run_name[46]; + sentry_uuid_as_string(&uuid, run_name); + + // `/.run` + strcpy(&run_name[36], ".run"); + sentry_path_t *run_path = sentry__path_join_str(database_path, run_name); + + // `/.run.lock` + strcpy(&run_name[40], ".lock"); + sentry_path_t *lock_path = sentry__path_join_str(database_path, run_name); + + sentry_run_t *run = run_new(database_path, run_path, lock_path); + if (run) { + run->uuid = uuid; + } + return run; +} + +sentry_run_t * +sentry__run_adopt( + const sentry_path_t *database_path, const sentry_path_t *run_path) +{ + if (!database_path || !run_path) { + return NULL; + } + sentry_path_t *owned_run_path = sentry__path_clone(run_path); + sentry_path_t *lock_path + = sentry__path_append_str(run_path, ".daemon.lock"); + return run_new(database_path, owned_run_path, lock_path); +} + bool sentry__run_should_skip_upload(sentry_run_t *run) { @@ -215,7 +237,9 @@ sentry__run_free(sentry_run_t *run) sentry__path_free(run->session_path); sentry__path_free(run->external_path); sentry__path_free(run->cache_path); - sentry__filelock_free(run->lock); + if (run->lock) { + sentry__filelock_free(run->lock); + } sentry_free(run->installation_id); sentry_free(run); } @@ -594,6 +618,26 @@ sentry__run_clear_session(const sentry_run_t *run) return !rv; } +void +sentry__process_run_envelopes( + const sentry_options_t *options, const sentry_path_t *run_path) +{ + sentry_pathiter_t *it = sentry__path_iter_directory(run_path); + const sentry_path_t *file; + while (it && (file = sentry__pathiter_next(it)) != NULL) { + if (!sentry__path_is_file(file) || sentry__path_is_symlink(file) + || !sentry__path_ends_with(file, ".envelope")) { + continue; + } + sentry_envelope_t *envelope = sentry__envelope_from_path(file); + if (envelope) { + sentry__capture_envelope(options->transport, envelope, options); + } + sentry__path_remove(file); + } + sentry__pathiter_free(it); +} + void sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash) { @@ -652,6 +696,28 @@ sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash) continue; } + sentry_path_t *daemon_lockfile + = sentry__path_append_str(run_dir, ".daemon.lock"); + sentry_filelock_t *daemon_lock = NULL; + if (!daemon_lockfile) { + sentry__filelock_free(lock); + continue; + } + if (sentry__path_is_file(daemon_lockfile)) { + daemon_lock = sentry__filelock_new(daemon_lockfile); + if (!daemon_lock || !sentry__filelock_try_lock(daemon_lock)) { + if (daemon_lock) { + sentry__filelock_free(daemon_lock); + } + sentry__filelock_free(lock); + continue; + } + } else { + sentry__path_free(daemon_lockfile); + } + + sentry__process_run_envelopes(options, run_dir); + sentry_pathiter_t *run_iter = sentry__path_iter_directory(run_dir); const sentry_path_t *file; while (run_iter && (file = sentry__pathiter_next(run_iter)) != NULL) { @@ -693,12 +759,6 @@ sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash) session_num = 0; } } - } else if (sentry__path_ends_with(file, ".envelope")) { - sentry_envelope_t *envelope = sentry__envelope_from_path(file); - if (envelope) { - sentry__capture_envelope( - options->transport, envelope, options); - } } sentry__path_remove(file); @@ -706,6 +766,9 @@ sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash) sentry__pathiter_free(run_iter); sentry__path_remove_all(run_dir); + if (daemon_lock) { + sentry__filelock_free(daemon_lock); + } sentry__filelock_free(lock); } sentry__pathiter_free(db_iter); diff --git a/src/sentry_database.h b/src/sentry_database.h index d1c94ece61..83d4e5e6e2 100644 --- a/src/sentry_database.h +++ b/src/sentry_database.h @@ -54,6 +54,14 @@ void sentry__run_load_installation_id(sentry_run_t *run, */ sentry_run_t *sentry__run_new(const sentry_path_t *database_path); +/** + * This creates a run object for a crash daemon that adopts an existing run + * directory. The daemon holds a separate lock so old-run processing waits for + * both the process and its daemon to finish. + */ +sentry_run_t *sentry__run_adopt( + const sentry_path_t *database_path, const sentry_path_t *run_path); + /** * Increment the refcount and return the run pointer. */ @@ -160,6 +168,12 @@ sentry_path_t *sentry__run_make_cache_path( void sentry__process_old_runs( const sentry_options_t *options, uint64_t last_crash); +/** + * Captures and removes all envelope files from a run directory. + */ +void sentry__process_run_envelopes( + const sentry_options_t *options, const sentry_path_t *run_path); + /** * Parses a cache filename in either form: * - `.envelope` sets `*ts_out = 0`, `*count_out = -1`. diff --git a/tests/assertions.py b/tests/assertions.py index 5520bdc359..b5cce57cbc 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -710,7 +710,7 @@ def wait_for_daemon(tmp_path, started_at, timeout=None): deadline = time.time() + timeout while time.time() < deadline: - for log_path in db_dir.glob("sentry-daemon-*.log"): + for log_path in db_dir.glob("*.run/sentry-daemon.log"): try: if log_path.stat().st_mtime < started_at: continue diff --git a/tests/test_e2e_sentry.py b/tests/test_e2e_sentry.py index 4a3877b4a7..785d9f7580 100644 --- a/tests/test_e2e_sentry.py +++ b/tests/test_e2e_sentry.py @@ -405,7 +405,7 @@ def print_daemon_logs(self): return # Find daemon log files - log_files = list(db_path.glob("sentry-daemon-*.log")) + log_files = list(db_path.glob("*.run/sentry-daemon.log")) if not log_files: print(f"\n=== No daemon log files found in {db_path} ===") return diff --git a/tests/test_integration_native.py b/tests/test_integration_native.py index 3e631b26f5..4cae17405e 100644 --- a/tests/test_integration_native.py +++ b/tests/test_integration_native.py @@ -188,12 +188,17 @@ def test_native_capture_minidump_generated(cmake, httpserver): ) assert waiting.result - # Check for minidump file in database directory + # Check for minidump file in run directory db_dir = tmp_path / ".sentry-native" assert db_dir.exists() - assert wait_for_file(db_dir / "*.dmp"), "Minidump file should be generated" - minidump_files = list(db_dir.glob("*.dmp")) + assert wait_for_file( + db_dir / "*.run/__sentry-crash.dmp" + ), "Minidump file should be generated" + minidump_files = list(db_dir.glob("*.run/__sentry-crash.dmp")) + assert not list(db_dir.glob("*.dmp")) + assert not list(db_dir.glob("sentry-daemon.log")) + assert not list(db_dir.glob("sentry-envelope-*")) # Verify minidump has correct header minidump_path = minidump_files[0] @@ -213,6 +218,16 @@ def test_native_capture_minidump_generated(cmake, httpserver): # Just verify it's non-zero assert version != 0, "Minidump should have non-zero version" + # The next SDK launch owns cleanup of the completed crash run. + run( + tmp_path, + "sentry_example", + ["log", "no-setup"], + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + ) + assert not list(db_dir.glob("*.run")) + assert not list(db_dir.glob("*.run*.lock")) + # Both daemon envelope writers merge the breadcrumb ring files: the native # stacktrace writer builds the event from scratch, while the minidump-only @@ -690,8 +705,8 @@ def test_native_minidump_streams(cmake, httpserver): # Find minidump db_dir = tmp_path / ".sentry-native" - assert wait_for_file(db_dir / "*.dmp") - minidump_files = list(db_dir.glob("*.dmp")) + assert wait_for_file(db_dir / "*.run/__sentry-crash.dmp") + minidump_files = list(db_dir.glob("*.run/__sentry-crash.dmp")) assert len(minidump_files) > 0 dump = _parse_minidump(minidump_files[0]) @@ -897,8 +912,8 @@ def test_native_smart_mode_captures_indirect_heap_memory(cmake, httpserver): assert waiting.result db_dir = tmp_path / ".sentry-native" - assert wait_for_file(db_dir / "*.dmp") - minidump_files = list(db_dir.glob("*.dmp")) + assert wait_for_file(db_dir / "*.run/__sentry-crash.dmp") + minidump_files = list(db_dir.glob("*.run/__sentry-crash.dmp")) assert len(minidump_files) > 0 dump = _parse_minidump(minidump_files[0]) @@ -946,6 +961,42 @@ def in_any(addr, ranges): ) +def test_native_uses_existing_run(cmake): + """The daemon adopts the existing run instead of creating root artifacts.""" + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "native"}) + exe = tmp_path / ( + "sentry_example.exe" if sys.platform == "win32" else "sentry_example" + ) + child = subprocess.Popen( + [str(exe), "log", "sleep"], + cwd=tmp_path, + env=dict(os.environ, SENTRY_DSN="https://foo@sentry.invalid/42"), + ) + db_dir = tmp_path / ".sentry-native" + + try: + assert wait_for( + lambda: len(list(db_dir.glob("*.run/sentry-daemon.log"))) == 1 + and len(list(db_dir.glob("*.run.daemon.lock"))) == 1 + ) + run_dirs = list(db_dir.glob("*.run")) + daemon_logs = list(db_dir.glob("*.run/sentry-daemon.log")) + + assert len(run_dirs) == 1 + assert daemon_logs[0].parent == run_dirs[0] + assert len(list(db_dir.glob("*.run.daemon.lock"))) == 1 + assert not list(db_dir.glob("sentry-daemon.log")) + assert not list(db_dir.glob("sentry-minidump-*")) + assert not list(db_dir.glob("sentry-envelope-*")) + assert not list(db_dir.glob("__sentry-stack*")) + assert not list(db_dir.glob("__sentry-modheaders")) + finally: + child.terminate() + child.wait() + # Windows cannot remove the run until the orphaned daemon releases it. + assert wait_for(lambda: not list(db_dir.glob("*.run.daemon.lock"))) + + def test_native_cleanup(cmake): """Test that cleanup works properly""" tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "native"}) @@ -960,6 +1011,10 @@ def test_native_cleanup(cmake): # Database should exist db_dir = tmp_path / ".sentry-native" assert db_dir.exists() + assert not list(db_dir.glob("*.run")) + assert not list(db_dir.glob("*.run*.lock")) + assert not list(db_dir.glob("sentry-daemon.log")) + assert not list(db_dir.glob("sentry-minidump-*")) def test_native_no_dsn_no_crash(cmake): @@ -977,7 +1032,7 @@ def test_native_no_dsn_no_crash(cmake): # Should not create database db_dir = tmp_path / ".sentry-native" if db_dir.exists(): - minidump_files = list(db_dir.glob("*.dmp")) + minidump_files = list(db_dir.glob("*.run/*.dmp")) # Minidumps might still be generated for debugging # but won't be uploaded diff --git a/tests/test_integration_tus.py b/tests/test_integration_tus.py index 2673cf09f2..a0480fba8a 100644 --- a/tests/test_integration_tus.py +++ b/tests/test_integration_tus.py @@ -578,4 +578,6 @@ def test_tus_crash_native(cmake, httpserver): for d in os.listdir(db_dir) if d.endswith(".run") and os.path.isdir(os.path.join(db_dir, d)) ] - assert run_dirs == [] + assert len(run_dirs) == 1 + run_dir = os.path.join(db_dir, run_dirs[0]) + assert not [f for f in os.listdir(run_dir) if f.endswith(".envelope")] diff --git a/tests/unit/test_native_backend.c b/tests/unit/test_native_backend.c index 0bf86275aa..89ab18c54b 100644 --- a/tests/unit/test_native_backend.c +++ b/tests/unit/test_native_backend.c @@ -5,7 +5,9 @@ * and low-level crash handling functionality. */ +#include "sentry_database.h" #include "sentry_options.h" +#include "sentry_path.h" #include "sentry_testsupport.h" #include @@ -19,6 +21,96 @@ # include "sentry_elf.h" #endif +SENTRY_TEST(daemon_adopts_existing_run) +{ +#if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) + SKIP_TEST(); +#endif + SENTRY_TEST_OPTIONS_NEW(options); + TEST_ASSERT(sentry__path_remove_all(options->database_path) == 0); + TEST_ASSERT(sentry__path_create_dir_all(options->database_path) == 0); + + sentry_run_t *existing_run = sentry__run_new(options->database_path); + TEST_ASSERT(!!existing_run); + sentry_run_t *adopted_run + = sentry__run_adopt(options->database_path, existing_run->run_path); + TEST_ASSERT(!!adopted_run); + + sentry_path_t *cache_path + = sentry__path_join_str(options->database_path, "cache"); + sentry_path_t *external_path + = sentry__path_join_str(options->database_path, "external"); + sentry_path_t *daemon_lock_path + = sentry__path_append_str(existing_run->run_path, ".daemon.lock"); + TEST_ASSERT(!!cache_path); + TEST_ASSERT(!!external_path); + TEST_ASSERT(!!daemon_lock_path); + + TEST_CHECK(sentry__path_eq(adopted_run->run_path, existing_run->run_path)); + TEST_CHECK(sentry__path_eq(adopted_run->cache_path, cache_path)); + TEST_CHECK(sentry__path_eq(adopted_run->external_path, external_path)); + TEST_CHECK(sentry__path_is_file(daemon_lock_path)); + + size_t run_count = 0; + sentry_pathiter_t *it = sentry__path_iter_directory(options->database_path); + const sentry_path_t *entry; + while (it && (entry = sentry__pathiter_next(it)) != NULL) { + if (sentry__path_is_dir(entry) + && sentry__path_ends_with(entry, ".run")) { + run_count++; + } + } + sentry__pathiter_free(it); + TEST_CHECK_INT_EQUAL(run_count, 1); + + sentry__run_free(adopted_run); + TEST_CHECK(!sentry__path_is_file(daemon_lock_path)); + sentry__run_clean(existing_run, true); + sentry__run_free(existing_run); + sentry__path_free(daemon_lock_path); + sentry__path_free(external_path); + sentry__path_free(cache_path); + sentry_options_free(options); +} + +SENTRY_TEST(daemon_run_blocks_old_run_cleanup) +{ +#if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) + SKIP_TEST(); +#endif + SENTRY_TEST_OPTIONS_NEW(options); + TEST_ASSERT(sentry__path_remove_all(options->database_path) == 0); + TEST_ASSERT(sentry__path_create_dir_all(options->database_path) == 0); + + options->run = sentry__run_new(options->database_path); + TEST_ASSERT(!!options->run); + sentry_run_t *old_run = sentry__run_new(options->database_path); + TEST_ASSERT(!!old_run); + sentry__filelock_unlock(old_run->lock); + + sentry_run_t *daemon_run + = sentry__run_adopt(options->database_path, old_run->run_path); + TEST_ASSERT(!!daemon_run); + sentry_path_t *artifact + = sentry__path_join_str(old_run->run_path, "daemon.log"); + TEST_ASSERT(!!artifact); + TEST_ASSERT(sentry__path_write_buffer(artifact, "log", 3) == 0); + + sentry__process_old_runs(options, 0); + TEST_CHECK(sentry__path_is_dir(old_run->run_path)); + TEST_CHECK(sentry__path_is_file(artifact)); + + sentry__run_free(daemon_run); + sentry__process_old_runs(options, 0); + TEST_CHECK(!sentry__path_is_dir(old_run->run_path)); + TEST_CHECK(!sentry__path_is_file(artifact)); + + sentry__path_free(artifact); + sentry__run_free(old_run); + sentry__run_clean(options->run, true); + sentry_options_free(options); +} + /** * Test minidump header structure size and alignment */ diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 12d5dd5b9e..fe24891b61 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -114,6 +114,8 @@ XX(crash_context_transport_fields) XX(crash_marker) XX(crashed_last_run) XX(custom_logger) +XX(daemon_adopts_existing_run) +XX(daemon_run_blocks_old_run_cleanup) XX(deserialize_envelope) XX(deserialize_envelope_empty) XX(deserialize_envelope_empty_attachments)