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.