common: avoid allocations for small numbers of traces.
What the developer wrote
Authored by Rusty Russell
common: avoid allocations for small numbers of traces.
If we only have 8 or fewer spans at once (as is the normal case), don't
do allocation, which might interfere with tracing.
This doesn't change our test_generate_coinmoves() benchmark.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This is a small internal cleanup in Core Lightning's tracing code. It replaces some heap memory allocations with a fixed set of pre-allocated slots for the common case of having 8 or fewer active trace spans. The change also adjusts how the memory-leak detector scans these objects because some are no longer allocated through the usual memory tracker. There is no direct security vulnerability visible in the diff; it is a performance and robustness improvement for developer/tracing instrumentation.
No immediate security action required. Treat as routine code maintenance. If reviewing, verify that memleak_scan_spans correctly marks all span objects and that the fixed_spans bounds check in trace_span_clear cannot be bypassed to free a static object.
Security signals we found
Memory management change in tracing subsystem
Custom memleak helper introduced for non-tal objects
No input validation, cryptography, or network parsing changes
No explicit security claim in commit message
Evidence from the diff
The patch modifies common/trace.c to keep a static array of 8 struct span objects (fixed_spans) and uses them before falling back to tal allocation. Because these static objects are not tal-allocated, the existing memleak_scan_htable helper would not find them, so the code switches from notleak_with_children to a custom memleak_add_helper callback (memleak_scan_spans) that iterates the span htable and marks each span and its region. The patch also adds a bounds check in trace_span_clear to distinguish fixed slots from dynamic ones and only tal_free the latter. The commit message frames this as avoiding allocations that ‘might interfere with tracing’.
Changed components
common/trace.ctracing / span allocationmemory leak detection integrationInspect captured patch +34 / −2
diff --git a/common/trace.c b/common/trace.c
index de42e2f4..6c1d607c 100644
--- a/common/trace.c
+++ b/common/trace.c
@@ -1,5 +1,6 @@
#include "config.h"
#include <assert.h>
+#include <ccan/array_size/array_size.h>
#include <ccan/crypto/siphash24/siphash24.h>
#include <ccan/endian/endian.h>
#include <ccan/err/err.h>
@@ -86,6 +87,7 @@ static bool span_key_eq(const struct span *span, size_t key)
HTABLE_DEFINE_NODUPS_TYPE(struct span, span_keyof, span_key_hash, span_key_eq,
span_htable);
+static struct span fixed_spans[8];
static struct span_htable *spans = NULL;
static struct span *current;
@@ -157,6 +159,19 @@ static void trace_inject_traceparent(void)
}
}
+static void memleak_scan_spans(struct htable *memtable, struct span_htable *spantable)
+{
+ struct span_htable_iter i;
+ const struct span *span;
+
+ for (span = span_htable_first(spantable, &i);
+ span;
+ span = span_htable_next(spantable, &i)) {
+ memleak_ptr(memtable, span);
+ memleak_scan_region(memtable, span, sizeof(*span));
+ }
+}
+
static void trace_init(void)
{
const char *dev_trace_file;
@@ -164,7 +179,10 @@ static void trace_init(void)
if (spans)
return;
- spans = notleak_with_children(tal(NULL, struct span_htable));
+ /* We can't use new_htable here because we put non-tal
+ * objects in our htable, and that breaks memleak_scan_htable! */
+ spans = notleak(tal(NULL, struct span_htable));
+ memleak_add_helper(spans, memleak_scan_spans);
span_htable_init(spans);
current = NULL;
@@ -197,7 +215,13 @@ static struct span *trace_span_find(size_t key)
*/
static struct span *trace_span_slot(void)
{
- /* FIXME: Try to avoid allocation! */
+ /* Look for a free fixed slot. */
+ for (size_t i = 0; i < ARRAY_SIZE(fixed_spans); i++) {
+ if (fixed_spans[i].key == 0)
+ return &fixed_spans[i];
+ }
+
+ /* Those are used up, we have to allocate. */
return tal(spans, struct span);
}
@@ -261,6 +285,14 @@ static void trace_span_clear(struct span *s)
{
if (!span_htable_del(spans, s))
abort();
+
+ /* If s is actually in fixed_spans, just zero it out. */
+ if (s >= fixed_spans && s < fixed_spans + ARRAY_SIZE(fixed_spans)) {
+ s->key = 0;
+ return;
+ }
+
+ /* Dynamically allocated, so we need to free it */
tal_free(s);
}
Why this scored 16/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.