libplugin: make timers have a "command" context.

This is cleaner: everything can now be associated with a command
context.

You're supposed to eventually dispose of it using timer_complete().

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2024-11-06 21:22:36 +10:30
parent 45f56f8e5d
commit 7ce30e2873
8 changed files with 136 additions and 83 deletions

View file

@ -206,7 +206,7 @@ static const struct subsystem_ops *get_subsystem_ops(const struct per_subsystem
} }
/* Mutual recursion */ /* Mutual recursion */
static void do_clean_timer(void *unused); static struct command_result *do_clean_timer(struct command *cmd, void *unused);
static struct command_result *do_clean(struct clean_info *cinfo); static struct command_result *do_clean(struct clean_info *cinfo);
static struct clean_info *new_clean_info(const tal_t *ctx, static struct clean_info *new_clean_info(const tal_t *ctx,
@ -267,7 +267,7 @@ static struct command_result *clean_finished(struct clean_info *cinfo)
} while (next_sv(&sv)); } while (next_sv(&sv));
/* autoclean-once? */ /* autoclean-once? */
if (cinfo->cmd) { if (cinfo != timer_cinfo) {
struct json_stream *response = jsonrpc_stream_success(cinfo->cmd); struct json_stream *response = jsonrpc_stream_success(cinfo->cmd);
json_object_start(response, "autoclean"); json_object_start(response, "autoclean");
@ -287,9 +287,10 @@ static struct command_result *clean_finished(struct clean_info *cinfo)
return command_finished(cinfo->cmd, response); return command_finished(cinfo->cmd, response);
} else { /* timer */ } else { /* timer */
plugin_log(plugin, LOG_DBG, "setting next timer"); plugin_log(plugin, LOG_DBG, "setting next timer");
cleantimer = plugin_timer(plugin, time_from_sec(cycle_seconds), cleantimer = global_timer(plugin,
time_from_sec(cycle_seconds),
do_clean_timer, NULL); do_clean_timer, NULL);
return timer_complete(plugin); return timer_complete(cinfo->cmd);
} }
} }
@ -554,7 +555,7 @@ static struct command_result *do_clean(struct clean_info *cinfo)
filter = tal_fmt(tmpctx, "{\"%s\":[{%s}]}", filter = tal_fmt(tmpctx, "{\"%s\":[{%s}]}",
ops->arr_name, ops->list_filter); ops->arr_name, ops->list_filter);
req = jsonrpc_request_with_filter_start(plugin, NULL, req = jsonrpc_request_with_filter_start(plugin, cinfo->cmd,
tal_fmt(tmpctx, tal_fmt(tmpctx,
"list%s", "list%s",
ops->system_name), ops->system_name),
@ -641,11 +642,12 @@ static struct command_result *start_clean(struct clean_info *cinfo)
} }
/* Needs a different signature than do_clean */ /* Needs a different signature than do_clean */
static void do_clean_timer(void *unused) static struct command_result *do_clean_timer(struct command *cmd, void *unused)
{ {
assert(timer_cinfo->cleanup_reqs_remaining == 0); assert(timer_cinfo->cleanup_reqs_remaining == 0);
cleantimer = NULL; cleantimer = NULL;
start_clean(timer_cinfo); timer_cinfo->cmd = cmd;
return start_clean(timer_cinfo);
} }
static struct command_result *param_subsystem(struct command *cmd, static struct command_result *param_subsystem(struct command *cmd,
@ -750,7 +752,7 @@ static const char *init(struct plugin *p,
tal_steal(plugin, timer_cinfo); tal_steal(plugin, timer_cinfo);
plugin_set_memleak_handler(plugin, memleak_mark_timer_cinfo); plugin_set_memleak_handler(plugin, memleak_mark_timer_cinfo);
cleantimer = plugin_timer(p, time_from_sec(cycle_seconds), do_clean_timer, NULL); cleantimer = global_timer(p, time_from_sec(cycle_seconds), do_clean_timer, NULL);
/* We don't care if this fails (it usually does, since entries /* We don't care if this fails (it usually does, since entries
* don't exist! */ * don't exist! */
@ -777,7 +779,8 @@ static char *cycle_seconds_option(struct plugin *plugin, const char *arg,
/* If timer is not running right now, reset it to new cycle_seconds */ /* If timer is not running right now, reset it to new cycle_seconds */
if (cleantimer) { if (cleantimer) {
tal_free(cleantimer); tal_free(cleantimer);
cleantimer = plugin_timer(plugin, time_from_sec(*cycle_seconds), cleantimer = global_timer(plugin,
time_from_sec(*cycle_seconds),
do_clean_timer, NULL); do_clean_timer, NULL);
} }
return NULL; return NULL;

View file

@ -199,15 +199,16 @@ static void destroy_bcli(struct bitcoin_cli *bcli)
list_del_from(&bitcoind->current, &bcli->list); list_del_from(&bitcoind->current, &bcli->list);
} }
static void retry_bcli(void *cb_arg) static struct command_result *retry_bcli(struct command *cmd,
struct bitcoin_cli *bcli)
{ {
struct bitcoin_cli *bcli = cb_arg;
list_del_from(&bitcoind->current, &bcli->list); list_del_from(&bitcoind->current, &bcli->list);
tal_del_destructor(bcli, destroy_bcli); tal_del_destructor(bcli, destroy_bcli);
list_add_tail(&bitcoind->pending[bcli->prio], &bcli->list); list_add_tail(&bitcoind->pending[bcli->prio], &bcli->list);
tal_free(bcli->output); tal_free(bcli->output);
next_bcli(bcli->prio); next_bcli(bcli->prio);
return timer_complete(cmd);
} }
/* We allow 60 seconds of spurious errors, eg. reorg. */ /* We allow 60 seconds of spurious errors, eg. reorg. */
@ -238,7 +239,7 @@ static void bcli_failure(struct bitcoin_cli *bcli,
bitcoind->error_count++; bitcoind->error_count++;
/* Retry in 1 second */ /* Retry in 1 second */
plugin_timer(bcli->cmd->plugin, time_from_sec(1), retry_bcli, bcli); command_timer(bcli->cmd, time_from_sec(1), retry_bcli, bcli);
} }
static void bcli_finished(struct io_conn *conn UNUSED, struct bitcoin_cli *bcli) static void bcli_finished(struct io_conn *conn UNUSED, struct bitcoin_cli *bcli)

View file

@ -397,11 +397,13 @@ static void destroy_sent(struct sent *sent)
} }
/* We've received neither a reply nor a payment; return failure. */ /* We've received neither a reply nor a payment; return failure. */
static void timeout_sent_invreq(struct sent *sent) static struct command_result *timeout_sent_invreq(struct command *timer_cmd,
struct sent *sent)
{ {
/* This will free sent! */ /* This will free sent! */
discard_result(command_fail(sent->cmd, OFFER_TIMEOUT, discard_result(command_fail(sent->cmd, OFFER_TIMEOUT,
"Timeout waiting for response")); "Timeout waiting for response"));
return timer_complete(timer_cmd);
} }
static struct command_result *sendonionmsg_done(struct command *cmd, static struct command_result *sendonionmsg_done(struct command *cmd,
@ -409,9 +411,9 @@ static struct command_result *sendonionmsg_done(struct command *cmd,
const jsmntok_t *result UNUSED, const jsmntok_t *result UNUSED,
struct sent *sent) struct sent *sent)
{ {
tal_steal(cmd, plugin_timer(cmd->plugin, command_timer(cmd,
time_from_sec(sent->wait_timeout), time_from_sec(sent->wait_timeout),
timeout_sent_invreq, sent)); timeout_sent_invreq, sent);
return command_still_pending(cmd); return command_still_pending(cmd);
} }
@ -618,7 +620,8 @@ static struct command_result *send_message(struct command *cmd,
} }
/* We've received neither a reply nor a payment; return failure. */ /* We've received neither a reply nor a payment; return failure. */
static void timeout_sent_inv(struct sent *sent) static struct command_result *timeout_sent_inv(struct command *timer_cmd,
struct sent *sent)
{ {
struct json_out *details = json_out_new(sent); struct json_out *details = json_out_new(sent);
@ -630,6 +633,7 @@ static void timeout_sent_inv(struct sent *sent)
discard_result(command_done_err(sent->cmd, OFFER_TIMEOUT, discard_result(command_done_err(sent->cmd, OFFER_TIMEOUT,
"Failed: timeout waiting for response", "Failed: timeout waiting for response",
details)); details));
return timer_complete(timer_cmd);
} }
static struct command_result *prepare_inv_timeout(struct command *cmd, static struct command_result *prepare_inv_timeout(struct command *cmd,
@ -637,9 +641,9 @@ static struct command_result *prepare_inv_timeout(struct command *cmd,
const jsmntok_t *result UNUSED, const jsmntok_t *result UNUSED,
struct sent *sent) struct sent *sent)
{ {
tal_steal(cmd, plugin_timer(cmd->plugin, command_timer(cmd,
time_from_sec(sent->wait_timeout), time_from_sec(sent->wait_timeout),
timeout_sent_inv, sent)); timeout_sent_inv, sent);
return sendonionmsg_done(cmd, buf, result, sent); return sendonionmsg_done(cmd, buf, result, sent);
} }

View file

@ -25,7 +25,8 @@
struct plugin_timer { struct plugin_timer {
struct timer timer; struct timer timer;
void (*cb)(void *cb_arg); const char *id;
struct command_result *(*cb)(struct command *cmd, void *cb_arg);
void *cb_arg; void *cb_arg;
}; };
@ -133,7 +134,6 @@ struct plugin {
STRMAP(const char *) usagemap; STRMAP(const char *) usagemap;
/* Timers */ /* Timers */
struct timers timers; struct timers timers;
size_t in_timer;
/* Feature set for lightningd */ /* Feature set for lightningd */
struct feature_set *our_features; struct feature_set *our_features;
@ -605,10 +605,10 @@ struct command_result *command_err_raw(struct command *cmd,
json_str, strlen(json_str)); json_str, strlen(json_str));
} }
struct command_result *timer_complete(struct plugin *p) struct command_result *timer_complete(struct command *cmd)
{ {
assert(p->in_timer > 0); assert(cmd->type == COMMAND_TYPE_TIMER);
p->in_timer--; tal_free(cmd);
return &complete; return &complete;
} }
@ -1677,11 +1677,14 @@ static void setup_command_usage(struct plugin *p)
static void call_plugin_timer(struct plugin *p, struct timer *timer) static void call_plugin_timer(struct plugin *p, struct timer *timer)
{ {
struct plugin_timer *t = container_of(timer, struct plugin_timer, timer); struct plugin_timer *t = container_of(timer, struct plugin_timer, timer);
struct command *timer_cmd;
struct command_result *res;
p->in_timer++; /* This *isn't* owned by timer, which is owned by original command,
/* Free this if they don't. */ * since they may free that in callback */
tal_steal(tmpctx, t); timer_cmd = new_command(p, p, t->id, "timer", COMMAND_TYPE_TIMER);
t->cb(t->cb_arg); res = t->cb(timer_cmd, t->cb_arg);
assert(res == &pending || res == &complete);
} }
static void destroy_plugin_timer(struct plugin_timer *timer, struct plugin *p) static void destroy_plugin_timer(struct plugin_timer *timer, struct plugin *p)
@ -1689,11 +1692,15 @@ static void destroy_plugin_timer(struct plugin_timer *timer, struct plugin *p)
timer_del(&p->timers, &timer->timer); timer_del(&p->timers, &timer->timer);
} }
struct plugin_timer *plugin_timer_(struct plugin *p, struct timerel t, static struct plugin_timer *new_timer(const tal_t *ctx,
void (*cb)(void *cb_arg), struct plugin *p,
void *cb_arg) const char *id TAKES,
struct timerel t,
struct command_result *(*cb)(struct command *, void *),
void *cb_arg)
{ {
struct plugin_timer *timer = notleak(tal(NULL, struct plugin_timer)); struct plugin_timer *timer = notleak(tal(ctx, struct plugin_timer));
timer->id = tal_strdup(timer, id);
timer->cb = cb; timer->cb = cb;
timer->cb_arg = cb_arg; timer->cb_arg = cb_arg;
timer_init(&timer->timer); timer_init(&timer->timer);
@ -1702,6 +1709,24 @@ struct plugin_timer *plugin_timer_(struct plugin *p, struct timerel t,
return timer; return timer;
} }
struct plugin_timer *global_timer_(struct plugin *p,
struct timerel t,
struct command_result *(*cb)(struct command *cmd, void *cb_arg),
void *cb_arg)
{
return new_timer(p, p, "timer", t, cb, cb_arg);
}
struct plugin_timer *command_timer_(struct command *cmd,
struct timerel t,
struct command_result *(*cb)(struct command *cmd, void *cb_arg),
void *cb_arg)
{
return new_timer(cmd, cmd->plugin,
take(tal_fmt(NULL, "%s-timer", cmd->id)),
t, cb, cb_arg);
}
void plugin_logv(struct plugin *p, enum log_level l, void plugin_logv(struct plugin *p, enum log_level l,
const char *fmt, va_list ap) const char *fmt, va_list ap)
{ {
@ -2280,7 +2305,6 @@ static struct plugin *new_plugin(const tal_t *ctx,
p->manifested = p->initialized = p->exiting = false; p->manifested = p->initialized = p->exiting = false;
p->restartability = restartability; p->restartability = restartability;
strmap_init(&p->usagemap); strmap_init(&p->usagemap);
p->in_timer = 0;
p->commands = commands; p->commands = commands;
if (taken(commands)) if (taken(commands))

View file

@ -433,27 +433,42 @@ struct command_result *forward_result(struct command *cmd,
/* Callback for timer where we expect a 'command_result'. All timers /* Callback for timer where we expect a 'command_result'. All timers
* must return this eventually, though they may do so via a convoluted * must return this eventually, though they may do so via a convoluted
* send_req() path. */ * send_req() path. */
struct command_result *timer_complete(struct plugin *p); struct command_result *timer_complete(struct command *cmd);
/* Signals that we've completed a command. Useful for when /* Signals that we've completed a command. Useful for when
* there's no `cmd` present */ * there's no `cmd` present. Deprecated! */
struct command_result *command_done(void); struct command_result *command_done(void);
/* Access timer infrastructure to add a timer. /* Access timer infrastructure to add a global timer for the plugin.
* *
* Freeing this releases the timer, otherwise it's freed after @cb * This is a timer with the same lifetime as the plugin.
* if it hasn't been freed already.
*/ */
struct plugin_timer *plugin_timer_(struct plugin *p, struct plugin_timer *global_timer_(struct plugin *p,
struct timerel t, struct timerel t,
void (*cb)(void *cb_arg), struct command_result *(*cb)(struct command *cmd, void *cb_arg),
void *cb_arg); void *cb_arg);
#define plugin_timer(plugin, time, cb, cb_arg) \ #define global_timer(plugin, time, cb, cb_arg) \
plugin_timer_((plugin), (time), \ global_timer_((plugin), (time), \
typesafe_cb(void, void *, \ typesafe_cb_preargs(struct command_result *, \
(cb), (cb_arg)), \ void *, \
(cb_arg)) \ (cb), (cb_arg), \
struct command *), \
(cb_arg)) \
/* Timer based off specific cmd */
struct plugin_timer *command_timer_(struct command *cmd,
struct timerel t,
struct command_result *(*cb)(struct command *cmd, void *cb_arg),
void *cb_arg);
#define command_timer(cmd, time, cb, cb_arg) \
command_timer_((cmd), (time), \
typesafe_cb_preargs(struct command_result *, \
void *, \
(cb), (cb_arg), \
struct command *), \
(cb_arg)) \
/* Log something */ /* Log something */
void plugin_log(struct plugin *p, enum log_level l, const char *fmt, ...) PRINTF_FMT(3, 4); void plugin_log(struct plugin *p, enum log_level l, const char *fmt, ...) PRINTF_FMT(3, 4);

View file

@ -28,9 +28,9 @@ static struct plugin_timer *lost_state_timer, *find_exes_timer, *peer_storage_ti
/* This tells if we are already in the process of recovery. */ /* This tells if we are already in the process of recovery. */
static bool recovery, already_has_peers; static bool recovery, already_has_peers;
static void do_check_lost_peer (void *unused); static struct command_result *do_check_lost_peer (struct command *cmd, void *unused);
static void do_check_gossip (struct command *cmd); static struct command_result *do_check_gossip (struct command *cmd, void *unused);
static void do_find_peer_storage (struct command *cmd); static struct command_result *do_find_peer_storage (struct command *cmd, void *unused);
static struct node_id local_id; static struct node_id local_id;
/* List of most connected nodes on the network */ /* List of most connected nodes on the network */
@ -84,9 +84,10 @@ static struct command_result *after_restorefrompeer(struct command *cmd,
plugin_log(plugin, LOG_DBG, "restorefrompeer called"); plugin_log(plugin, LOG_DBG, "restorefrompeer called");
peer_storage_timer = peer_storage_timer =
plugin_timer(plugin, time_from_sec(CHECK_STORAGE_INTERVAL), global_timer(plugin,
do_find_peer_storage, cmd); time_from_sec(CHECK_STORAGE_INTERVAL),
return command_still_pending(cmd); do_find_peer_storage, NULL);
return timer_complete(cmd);
} }
static struct command_result *find_peer_storage (struct command *cmd) static struct command_result *find_peer_storage (struct command *cmd)
@ -101,14 +102,13 @@ static struct command_result *find_peer_storage (struct command *cmd)
return send_outreq(plugin, req); return send_outreq(plugin, req);
} }
static void do_find_peer_storage (struct command *cmd) static struct command_result *do_find_peer_storage(struct command *cmd, void *unused)
{ {
find_peer_storage(cmd); return find_peer_storage(cmd);
return;
} }
static void do_check_gossip (struct command *cmd) static struct command_result *do_check_gossip(struct command *cmd, void *unused)
{ {
find_exes_timer = NULL; find_exes_timer = NULL;
@ -144,14 +144,15 @@ static void do_check_gossip (struct command *cmd)
} }
peer_storage_timer = peer_storage_timer =
plugin_timer(plugin, time_from_sec(CHECK_STORAGE_INTERVAL), global_timer(plugin,
do_find_peer_storage, cmd); time_from_sec(CHECK_STORAGE_INTERVAL),
return; do_find_peer_storage, NULL);
return timer_complete(cmd);
} }
find_exes_timer = plugin_timer( find_exes_timer = global_timer(
plugin, time_from_sec(CHECK_PEER_INTERVAL), do_check_gossip, cmd); plugin, time_from_sec(CHECK_PEER_INTERVAL), do_check_gossip, NULL);
return; return timer_complete(cmd);
} }
static void entering_recovery_mode(struct command *cmd) static void entering_recovery_mode(struct command *cmd)
@ -182,7 +183,7 @@ static void entering_recovery_mode(struct command *cmd)
NULL); NULL);
send_outreq(plugin, req_emer_recovery); send_outreq(plugin, req_emer_recovery);
find_exes_timer = plugin_timer( find_exes_timer = global_timer(
plugin, time_from_sec(CHECK_GOSSIP_INTERVAL), do_check_gossip, cmd); plugin, time_from_sec(CHECK_GOSSIP_INTERVAL), do_check_gossip, cmd);
return; return;
} }
@ -216,32 +217,32 @@ static struct command_result *after_listpeerchannels(struct command *cmd,
} }
lost_state_timer = lost_state_timer =
plugin_timer(plugin, time_from_sec(CHECK_PEER_INTERVAL), global_timer(plugin, time_from_sec(CHECK_PEER_INTERVAL),
do_check_lost_peer, NULL); do_check_lost_peer, NULL);
return command_still_pending(cmd); return command_still_pending(cmd);
} }
static struct command_result *check_lost_peer(void *unused) static struct command_result *check_lost_peer(struct command *cmd)
{ {
struct out_req *req; struct out_req *req;
req = jsonrpc_request_start(plugin, NULL, "listpeerchannels", req = jsonrpc_request_start(plugin, cmd, "listpeerchannels",
after_listpeerchannels, after_listpeerchannels,
&forward_error, NULL); &forward_error, NULL);
return send_outreq(plugin, req); return send_outreq(plugin, req);
} }
static void do_check_lost_peer (void *unused) static struct command_result *do_check_lost_peer(struct command *cmd, void *unused)
{ {
/* Set to NULL when already in progress. */ /* Set to NULL when already in progress. */
lost_state_timer = NULL; lost_state_timer = NULL;
if (recovery) { if (recovery) {
return; return timer_complete(cmd);
} }
check_lost_peer(unused); return check_lost_peer(cmd);
} }
static const char *init(struct plugin *p, static const char *init(struct plugin *p,
@ -251,7 +252,7 @@ static const char *init(struct plugin *p,
plugin = p; plugin = p;
plugin_log(p, LOG_DBG, "Recover Plugin Initialised!"); plugin_log(p, LOG_DBG, "Recover Plugin Initialised!");
recovery = false; recovery = false;
lost_state_timer = plugin_timer(plugin, time_from_sec(STARTUP_TIME), lost_state_timer = global_timer(plugin, time_from_sec(STARTUP_TIME),
do_check_lost_peer, NULL); do_check_lost_peer, NULL);
u32 num_peers; u32 num_peers;
size_t num_cupdates_rejected; size_t num_cupdates_rejected;

View file

@ -771,21 +771,24 @@ REGISTER_PAYMENT_MODIFIER(send_routes, send_routes_cb);
* The payment main thread sleeps for some time. * The payment main thread sleeps for some time.
*/ */
static void sleep_done(struct payment *payment) static struct command_result *sleep_done(struct command *cmd, struct payment *payment)
{ {
struct command_result *ret;
payment->waitresult_timer = NULL; payment->waitresult_timer = NULL;
// TODO: is this compulsory? ret = timer_complete(cmd);
timer_complete(pay_plugin->plugin);
payment_continue(payment); payment_continue(payment);
return ret;
} }
static struct command_result *sleep_cb(struct payment *payment) static struct command_result *sleep_cb(struct payment *payment)
{ {
assert(payment->waitresult_timer == NULL);
payment->waitresult_timer = plugin_timer(
pay_plugin->plugin, time_from_msec(COLLECTOR_TIME_WINDOW_MSEC), sleep_done, payment);
struct command *cmd = payment_command(payment); struct command *cmd = payment_command(payment);
assert(cmd); assert(cmd);
assert(payment->waitresult_timer == NULL);
payment->waitresult_timer
= command_timer(cmd,
time_from_msec(COLLECTOR_TIME_WINDOW_MSEC),
sleep_done, payment);
return command_still_pending(cmd); return command_still_pending(cmd);
} }

View file

@ -1649,10 +1649,12 @@ perform_multiconnect(struct multifundchannel_command *mfc)
/* Initiate the multifundchannel execution. */ /* Initiate the multifundchannel execution. */
static void static struct command_result *
perform_multifundchannel(struct multifundchannel_command *mfc) perform_multifundchannel(struct command *timer_cmd,
struct multifundchannel_command *mfc)
{ {
perform_multiconnect(mfc); perform_multiconnect(mfc);
return timer_complete(timer_cmd);
} }
@ -1778,8 +1780,8 @@ post_cleanup_redo_multifundchannel(struct multifundchannel_redo *redo)
/* Okay, we still have destinations to try: wait a second in case it /* Okay, we still have destinations to try: wait a second in case it
* takes that long to disconnect from peer, then retry. */ * takes that long to disconnect from peer, then retry. */
plugin_timer(mfc->cmd->plugin, time_from_sec(1), command_timer(mfc->cmd, time_from_sec(1),
perform_multifundchannel, mfc); perform_multifundchannel, mfc);
return command_still_pending(mfc->cmd); return command_still_pending(mfc->cmd);
} }
@ -2011,7 +2013,7 @@ json_multifundchannel(struct command *cmd,
mfc->sigs_collected = false; mfc->sigs_collected = false;
perform_multifundchannel(mfc); perform_multiconnect(mfc);
return command_still_pending(mfc->cmd); return command_still_pending(mfc->cmd);
} }