From 52887a20ee14b6481240400806121e7bb54ce778 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 3 Apr 2025 07:03:59 +1030 Subject: [PATCH] trace: handle key being freed while suspended. This happens with autoclean, which does a datastore request then frees the parent command without waiting for a response (see clean_finished). This leaks a trace, and causes a crash if the pointer is later reused. My solution is to create a trace variant which declares the trace key to be a tal ptr and then we can clean up in the destructor if this happens. This fixes the issue for me. Signed-off-by: Rusty Russell Changelog-Fixed: autoclean: fixed occasional crash when tracepoints compiled in. --- common/trace.c | 23 +++++++++++++++++++++++ common/trace.h | 2 ++ plugins/libplugin.c | 2 +- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/common/trace.c b/common/trace.c index 9e7a01c863d3..5bd5fa5647fa 100644 --- a/common/trace.c +++ b/common/trace.c @@ -373,6 +373,28 @@ void trace_span_suspend_(const void *key, const char *lbl) DTRACE_PROBE1(lightningd, span_suspend, span->id); } +static void destroy_trace_span(const void *key) +{ + size_t numkey = trace_key(key); + struct span *span = trace_span_find(numkey); + + /* It's usually ended normally. */ + if (!span) + return; + + /* Otherwise resume so we can terminate it */ + trace_span_resume(key); + trace_span_end(key); +} + +void trace_span_suspend_may_free_(const void *key, const char *lbl) +{ + if (disable_trace) + return; + trace_span_suspend_(key, lbl); + tal_add_destructor(key, destroy_trace_span); +} + void trace_span_resume_(const void *key, const char *lbl) { if (disable_trace) @@ -395,6 +417,7 @@ void trace_cleanup(void) void trace_span_start(const char *name, const void *key) {} void trace_span_end(const void *key) {} void trace_span_suspend_(const void *key, const char *lbl) {} +void trace_span_suspend_may_free_(const void *key, const char *lbl) {} void trace_span_resume_(const void *key, const char *lbl) {} void trace_span_tag(const void *key, const char *name, const char *value) {} void trace_cleanup(void) {} diff --git a/common/trace.h b/common/trace.h index 4fc8a4149e8a..d8996684b9d7 100644 --- a/common/trace.h +++ b/common/trace.h @@ -15,8 +15,10 @@ void trace_span_remote(u8 trace_id[TRACE_ID_SIZE], u8 span_id[SPAN_ID_SIZE]); #define TRACE_LBL __FILE__ ":" stringify(__LINE__) void trace_span_suspend_(const void *key, const char *lbl); +void trace_span_suspend_may_free_(const void *key, const char *lbl); void trace_span_resume_(const void *key, const char *lbl); #define trace_span_suspend(key) trace_span_suspend_(key, TRACE_LBL) +#define trace_span_suspend_may_free(key) trace_span_suspend_may_free_(key, TRACE_LBL) #define trace_span_resume(key) trace_span_resume_(key, TRACE_LBL) #endif /* LIGHTNING_COMMON_TRACE_H */ diff --git a/plugins/libplugin.c b/plugins/libplugin.c index b23baade672b..b5f483720719 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -1110,7 +1110,7 @@ send_outreq(const struct out_req *req) * callback. */ trace_span_start("jsonrpc", req); trace_span_tag(req, "id", req->id); - trace_span_suspend(req); + trace_span_suspend_may_free(req); ld_rpc_send(req->cmd->plugin, req->js); notleak_with_children(req->cmd);