Skip to content

Commit 2e6ad3f

Browse files
rustyrussellendothermicdev
authored andcommitted
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 <rusty@rustcorp.com.au> Changelog-Fixed: autoclean: fixed occasional crash when tracepoints compiled in.
1 parent 7c6270d commit 2e6ad3f

File tree

3 files changed

+26
-1
lines changed

3 files changed

+26
-1
lines changed

common/trace.c

+23
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,28 @@ void trace_span_suspend_(const void *key, const char *lbl)
373373
DTRACE_PROBE1(lightningd, span_suspend, span->id);
374374
}
375375

376+
static void destroy_trace_span(const void *key)
377+
{
378+
size_t numkey = trace_key(key);
379+
struct span *span = trace_span_find(numkey);
380+
381+
/* It's usually ended normally. */
382+
if (!span)
383+
return;
384+
385+
/* Otherwise resume so we can terminate it */
386+
trace_span_resume(key);
387+
trace_span_end(key);
388+
}
389+
390+
void trace_span_suspend_may_free_(const void *key, const char *lbl)
391+
{
392+
if (disable_trace)
393+
return;
394+
trace_span_suspend_(key, lbl);
395+
tal_add_destructor(key, destroy_trace_span);
396+
}
397+
376398
void trace_span_resume_(const void *key, const char *lbl)
377399
{
378400
if (disable_trace)
@@ -395,6 +417,7 @@ void trace_cleanup(void)
395417
void trace_span_start(const char *name, const void *key) {}
396418
void trace_span_end(const void *key) {}
397419
void trace_span_suspend_(const void *key, const char *lbl) {}
420+
void trace_span_suspend_may_free_(const void *key, const char *lbl) {}
398421
void trace_span_resume_(const void *key, const char *lbl) {}
399422
void trace_span_tag(const void *key, const char *name, const char *value) {}
400423
void trace_cleanup(void) {}

common/trace.h

+2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@ void trace_span_remote(u8 trace_id[TRACE_ID_SIZE], u8 span_id[SPAN_ID_SIZE]);
1515

1616
#define TRACE_LBL __FILE__ ":" stringify(__LINE__)
1717
void trace_span_suspend_(const void *key, const char *lbl);
18+
void trace_span_suspend_may_free_(const void *key, const char *lbl);
1819
void trace_span_resume_(const void *key, const char *lbl);
1920
#define trace_span_suspend(key) trace_span_suspend_(key, TRACE_LBL)
21+
#define trace_span_suspend_may_free(key) trace_span_suspend_may_free_(key, TRACE_LBL)
2022
#define trace_span_resume(key) trace_span_resume_(key, TRACE_LBL)
2123

2224
#endif /* LIGHTNING_COMMON_TRACE_H */

plugins/libplugin.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -1110,7 +1110,7 @@ send_outreq(const struct out_req *req)
11101110
* callback. */
11111111
trace_span_start("jsonrpc", req);
11121112
trace_span_tag(req, "id", req->id);
1113-
trace_span_suspend(req);
1113+
trace_span_suspend_may_free(req);
11141114

11151115
ld_rpc_send(req->cmd->plugin, req->js);
11161116
notleak_with_children(req->cmd);

0 commit comments

Comments
 (0)