libplugin: remove redundant destructor which causes exponential slowdown on large numbers of requests.
What the developer wrote
Authored by Rusty Russell
libplugin: remove redundant destructor which causes exponential slowdown on large numbers of requests.
Note that we create a destructor on the command to reset request->cmd
pointer if the cmd is freed (so we know not to call the callback).
But attaching hundreds of thousands of them is slow: it's a
single-linked list, which is iterated in several places.
But that's redundant: the request is now allocated off the cmd, so freeing the command
will free the request anyway.
Hacking in something to print progress to a file, here's the number of
requests processed every 10 seconds before and after:
Before:
$ while sleep 10; do wc -l /tmp/bkpr-progress; done
181529 /tmp/bkpr-progress
195994 /tmp/bkpr-progress
207083 /tmp/bkpr-progress
226336 /tmp/bkpr-progress
234319 /tmp/bkpr-progress
241514 /tmp/bkpr-progress
247421 /tmp/bkpr-progress
255292 /tmp/bkpr-progress
261367 /tmp/bkpr-progress
269085 /tmp/bkpr-progress
276953 /tmp/bkpr-progress
282233 /tmp/bkpr-progress
286193 /tmp/bkpr-progress
290930 /tmp/bkpr-progress
295276 /tmp/bkpr-progress
301086 /tmp/bkpr-progress
After:
169505 /tmp/bkpr-progress
196010 /tmp/bkpr-progress
219370 /tmp/bkpr-progress
235671 /tmp/bkpr-progress
244242 /tmp/bkpr-progress
255362 /tmp/bkpr-progress
265636 /tmp/bkpr-progress
276966 /tmp/bkpr-progress
284451 /tmp/bkpr-progress
288836 /tmp/bkpr-progress
296578 /tmp/bkpr-progress
304571 /tmp/bkpr-progress
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This commit fixes a performance bug, not a security vulnerability. It removes an unnecessary cleanup step that was attached to every outgoing plugin request. On large numbers of requests, that extra step created a very long internal list and slowed everything down dramatically. The fix simply relies on normal memory cleanup instead.
Treat as a routine performance fix. No security patch urgency, but operators running plugins that issue very large numbers of RPC requests (e.g., bookkeeping) should upgrade to avoid slowdowns. No workaround needed.
Security signals we found
Performance degradation / denial-of-service potential from O(n^2) destructor-list behavior under high request volume
No memory corruption, type confusion, authentication bypass, or cryptographic weakness present in diff
Redundant lifecycle management removed; behavior is functionally equivalent because request is child of cmd
Evidence from the diff
The patch removes a tal destructor (disable_request_cb) registered on out->cmd for every outgoing JSON-RPC request in plugins/libplugin.c. The destructor was meant to null out request->cmd and disable callbacks if the parent command was freed early. However, the request is already allocated off the cmd, so freeing cmd frees the request anyway, making the destructor redundant. The destructor added an entry to a singly-linked list per request; with hundreds of thousands of requests, traversal of that list caused exponential slowdown. The patch deletes the destructor registration, the removal call, and the now-unused helper functions ignore_cb and disable_request_cb.
Changed components
plugins/libplugin.cOutgoing JSON-RPC request lifecycle in CLN pluginstal destructor list managementInspect captured patch +0 / −25
diff --git a/plugins/libplugin.c b/plugins/libplugin.c
index 36a74d9f..167d1362 100644
--- a/plugins/libplugin.c
+++ b/plugins/libplugin.c
@@ -290,17 +290,6 @@ static void ld_rpc_send(struct plugin *plugin, struct json_stream *stream)
io_wake(plugin->io_rpc_conn);
}
-
-/* When cmd for request is gone, we use this as noop callback */
-static struct command_result *ignore_cb(struct command *command,
- const char *method,
- const char *buf,
- const jsmntok_t *result,
- void *arg)
-{
- return &complete;
-}
-
/* Ignore the result, and terminate the timer/aux/hook */
struct command_result *ignore_and_complete(struct command *cmd,
const char *method,
@@ -357,14 +346,6 @@ struct command_result *plugin_broken_cb(struct command *cmd,
json_tok_full(buf, result));
}
-static void disable_request_cb(struct command *cmd, struct out_req *out)
-{
- out->errcb = NULL;
- out->cb = ignore_cb;
- /* Called because cmd got free'd */
- out->cmd = NULL;
-}
-
/* Prefix is usually a cmd->id */
static const char *json_id(const tal_t *ctx, struct plugin *plugin,
const char *method, const char *prefix)
@@ -424,9 +405,6 @@ jsonrpc_request_start_(struct command *cmd,
strmap_add(&cmd->plugin->out_reqs, out->id, out);
tal_add_destructor2(out, destroy_out_req, cmd->plugin);
- /* If command goes away, don't call callbacks! */
- tal_add_destructor2(out->cmd, disable_request_cb, out);
-
out->js = new_json_stream(NULL, cmd, NULL);
json_object_start(out->js, NULL);
json_add_string(out->js, "jsonrpc", "2.0");
@@ -1100,9 +1078,6 @@ static void handle_rpc_reply(struct plugin *plugin, const char *buf, const jsmnt
return;
}
- /* Remove destructor if one existed */
- tal_del_destructor2(out->cmd, disable_request_cb, out);
-
/* We want to free this if callback doesn't. */
tal_steal(tmpctx, out);
Why this scored 25/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.