From 3a95705f9003f5e814c0de211471bfa374d567ca Mon Sep 17 00:00:00 2001 From: Adam Soltys Date: Fri, 16 Oct 2026 07:19:11 -0700 Subject: [PATCH] askrene: don't crash if a layer is removed while getroutes waits Layer names passed to getroutes are checked when the command arrives, but they are only looked up again in apply_layers() once a child is free (up to askrene-max-threads at once). If the layer was removed in between, find_layer() returns NULL and we hit the assert that the only remaining possibility is auto.sourcefree, killing the plugin and lightningd with it. This is easy to hit with the common pattern of creating a temporary layer, calling getroutes, and removing the layer when the call returns or times out on the client side: while askrene is busy, the request is still queued when the cleanup runs. Fail that request with an error instead. Changelog-Fixed: askrene: `getroutes` no longer crashes lightningd if one of its layers is removed while the request is waiting to run. Signed-off-by: Adam Soltys Co-Authored-By: Claude Opus 5.5 --- plugins/askrene/askrene.c | 23 +++++++++++++++++------ tests/test_askrene.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/plugins/askrene/askrene.c b/plugins/askrene/askrene.c index ab795e17ec67..b532d69377c5 100644 --- a/plugins/askrene/askrene.c +++ b/plugins/askrene/askrene.c @@ -376,7 +376,10 @@ static void add_layer(const struct layer ***layers, layer_clear_overridden_capacities(l, gossmap, capacities); } -/* Gather layers, clear capacities where layers contains info */ +/* Gather layers, clear capacities where layers contains info. + * Returns NULL (and sets *unknown) if a layer no longer exists: layer + * names are only checked when the command arrives, and the layer can be + * removed while the request is waiting for a free child. */ static const struct layer **apply_layers(const tal_t *ctx, struct askrene *askrene, struct command *cmd, @@ -384,7 +387,8 @@ static const struct layer **apply_layers(const tal_t *ctx, struct gossmap_localmods *localmods, const char **layernames, const struct layer *local_layer, - fp16_t *capacities) + fp16_t *capacities, + const char **unknown) { const struct layer **layers = tal_arr(ctx, const struct layer *, 0); /* Layers must exist, but might be special ones! */ @@ -402,10 +406,12 @@ static const struct layer **apply_layers(const tal_t *ctx, /* This layer takes effect when converting flows * into routes. */ continue; - } else { - assert(streq(layernames[i], "auto.sourcefree")); + } else if (streq(layernames[i], "auto.sourcefree")) { cmd_log(tmpctx, cmd, LOG_DBG, "Adding auto.sourcefree"); l = source_free_layer(layernames, askrene, source, localmods); + } else { + *unknown = layernames[i]; + return tal_free(layers); } } add_layer(&layers, l, askrene->gossmap, localmods, capacities); @@ -553,6 +559,7 @@ static struct command_result *do_getroutes(struct command *cmd, int replyfds[2], logfds[2]; struct router_child *child; const struct layer **layers; + const char *unknown_layer; s8 *biases; fp16_t *capacities; bool include_next_node_id, include_amount_msat, include_delay; @@ -600,8 +607,12 @@ static struct command_result *do_getroutes(struct command *cmd, /* apply selected layers to the localmods */ layers = apply_layers(cmd, askrene, cmd, &info->source, localmods, - info->layers, info->local_layer, capacities); - + info->layers, info->local_layer, capacities, + &unknown_layer); + if (!layers) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "Layer %s was removed before routing began", + unknown_layer); /* no parallel payments means we can drop any smaller channels */ if (info->maxparts == 1) { diff --git a/tests/test_askrene.py b/tests/test_askrene.py index bbf07e63f7c9..fcdcf0549ea3 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -1714,6 +1714,37 @@ def test_real_data(node_factory, bitcoind, executor): assert l1.daemon.is_in_log(r"Too many running at once \(4 vs 4\): waiting") +def test_layer_removed_while_waiting(node_factory, executor): + """A layer removed while getroutes waits for a free child must not crash askrene""" + gsfile, nodemap = generate_gossip_store([GenChannel(0, 1), + GenChannel(1, 2)]) + l1 = node_factory.get_node(gossip_store_file=gsfile.name) + + def getroutes(layers): + return l1.rpc.getroutes(source=nodemap[0], + destination=nodemap[2], + amount_msat=1000, + layers=layers, + maxfee_msat=1000, + final_cltv=99) + + # With no children allowed, the request has to wait. + l1.rpc.askrene_create_layer('vanishing') + l1.rpc.setconfig('askrene-max-threads', 0) + waiting = executor.submit(getroutes, ['vanishing']) + l1.daemon.wait_for_log(r"Too many running at once \(0 vs 0\): waiting") + l1.rpc.askrene_remove_layer('vanishing') + + # Finishing another request wakes the waiting one. + l1.rpc.setconfig('askrene-max-threads', 1) + getroutes([]) + with pytest.raises(RpcError, match="Layer vanishing was removed before routing began"): + waiting.result(TIMEOUT) + + # askrene is still alive. + getroutes([]) + + @pytest.mark.slow_test def test_real_biases(node_factory, bitcoind, executor): # Route from Rusty's node to the top 100.