plugin: simplify hooks calling methods, and make lifetime requirements explicit.

They callback must take ownership of the payload (almost all do, but
now it's explicit).

And since the payload and cb_arg arguments to plugin_hook_call_() are
always identical, make them a single parameter.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2020-04-15 19:50:41 +09:30
parent 1e34d8989d
commit 9aedb0c61f
11 changed files with 44 additions and 49 deletions

View File

@ -231,7 +231,7 @@ static const u8 *hook_gives_failmsg(const tal_t *ctx,
} }
static void static void
invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload, invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload STEALS,
const char *buffer, const char *buffer,
const jsmntok_t *toks) const jsmntok_t *toks)
{ {
@ -281,7 +281,6 @@ invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload,
REGISTER_PLUGIN_HOOK(invoice_payment, REGISTER_PLUGIN_HOOK(invoice_payment,
PLUGIN_HOOK_SINGLE, PLUGIN_HOOK_SINGLE,
invoice_payment_hook_cb, invoice_payment_hook_cb,
struct invoice_payment_hook_payload *,
invoice_payment_serialize, invoice_payment_serialize,
struct invoice_payment_hook_payload *); struct invoice_payment_hook_payload *);
@ -360,7 +359,7 @@ void invoice_try_pay(struct lightningd *ld,
{ {
struct invoice_payment_hook_payload *payload; struct invoice_payment_hook_payload *payload;
payload = tal(ld, struct invoice_payment_hook_payload); payload = tal(NULL, struct invoice_payment_hook_payload);
payload->ld = ld; payload->ld = ld;
payload->label = tal_steal(payload, details->label); payload->label = tal_steal(payload, details->label);
payload->msat = set->so_far; payload->msat = set->so_far;
@ -368,7 +367,7 @@ void invoice_try_pay(struct lightningd *ld,
payload->set = set; payload->set = set;
tal_add_destructor2(set, invoice_payload_remove_set, payload); tal_add_destructor2(set, invoice_payload_remove_set, payload);
plugin_hook_call_invoice_payment(ld, payload, payload); plugin_hook_call_invoice_payment(ld, payload);
} }
static bool hsm_sign_b11(const u5 *u5bytes, static bool hsm_sign_b11(const u5 *u5bytes,

View File

@ -29,6 +29,7 @@
#include <common/memleak.h> #include <common/memleak.h>
#include <common/param.h> #include <common/param.h>
#include <common/timeout.h> #include <common/timeout.h>
#include <common/utils.h>
#include <common/version.h> #include <common/version.h>
#include <common/wireaddr.h> #include <common/wireaddr.h>
#include <errno.h> #include <errno.h>
@ -676,13 +677,16 @@ fail:
} }
static void static void
rpc_command_hook_callback(struct rpc_command_hook_payload *p, rpc_command_hook_callback(struct rpc_command_hook_payload *p STEALS,
const char *buffer, const jsmntok_t *resulttok) const char *buffer, const jsmntok_t *resulttok)
{ {
const jsmntok_t *tok, *params, *custom_return; const jsmntok_t *tok, *params, *custom_return;
const jsmntok_t *innerresulttok; const jsmntok_t *innerresulttok;
struct json_stream *response; struct json_stream *response;
/* Free payload with cmd */
tal_steal(p->cmd, p);
params = json_get_member(p->buffer, p->request, "params"); params = json_get_member(p->buffer, p->request, "params");
/* If no plugin registered, just continue command execution. Same if /* If no plugin registered, just continue command execution. Same if
@ -767,13 +771,12 @@ rpc_command_hook_callback(struct rpc_command_hook_payload *p,
REGISTER_PLUGIN_HOOK(rpc_command, PLUGIN_HOOK_SINGLE, REGISTER_PLUGIN_HOOK(rpc_command, PLUGIN_HOOK_SINGLE,
rpc_command_hook_callback, rpc_command_hook_callback,
struct rpc_command_hook_payload *,
rpc_command_hook_serialize, rpc_command_hook_serialize,
struct rpc_command_hook_payload *); struct rpc_command_hook_payload *);
static void call_rpc_command_hook(struct rpc_command_hook_payload *p) static void call_rpc_command_hook(struct rpc_command_hook_payload *p)
{ {
plugin_hook_call_rpc_command(p->cmd->ld, p, p); plugin_hook_call_rpc_command(p->cmd->ld, p);
} }
/* We return struct command_result so command_fail return value has a natural /* We return struct command_result so command_fail return value has a natural
@ -842,7 +845,7 @@ parse_request(struct json_connection *jcon, const jsmntok_t tok[])
jcon->buffer + method->start); jcon->buffer + method->start);
} }
rpc_hook = tal(c, struct rpc_command_hook_payload); rpc_hook = tal(NULL, struct rpc_command_hook_payload);
rpc_hook->cmd = c; rpc_hook->cmd = c;
/* Duplicate since we might outlive the connection */ /* Duplicate since we might outlive the connection */
rpc_hook->buffer = tal_dup_talarr(rpc_hook, char, jcon->buffer); rpc_hook->buffer = tal_dup_talarr(rpc_hook, char, jcon->buffer);

View File

@ -41,7 +41,7 @@ onion_message_serialize(struct onion_message_hook_payload *payload,
} }
static void static void
onion_message_hook_cb(struct onion_message_hook_payload *payload, onion_message_hook_cb(struct onion_message_hook_payload *payload STEALS,
const char *buffer, const char *buffer,
const jsmntok_t *toks) const jsmntok_t *toks)
{ {
@ -53,7 +53,6 @@ onion_message_hook_cb(struct onion_message_hook_payload *payload,
REGISTER_PLUGIN_HOOK(onion_message, REGISTER_PLUGIN_HOOK(onion_message,
PLUGIN_HOOK_CHAIN, PLUGIN_HOOK_CHAIN,
onion_message_hook_cb, onion_message_hook_cb,
struct onion_message_hook_payload *,
onion_message_serialize, onion_message_serialize,
struct onion_message_hook_payload *); struct onion_message_hook_payload *);
@ -112,7 +111,7 @@ void handle_onionmsg_to_us(struct channel *channel, const u8 *msg)
log_debug(channel->log, "Got onionmsg%s%s", log_debug(channel->log, "Got onionmsg%s%s",
payload->reply_blinding ? " reply_blinding": "", payload->reply_blinding ? " reply_blinding": "",
payload->reply_path ? " reply_path": ""); payload->reply_path ? " reply_path": "");
plugin_hook_call_onion_message(ld, payload, payload); plugin_hook_call_onion_message(ld, payload);
} }
void handle_onionmsg_forward(struct channel *channel, const u8 *msg) void handle_onionmsg_forward(struct channel *channel, const u8 *msg)

View File

@ -754,7 +754,7 @@ static void openchannel_payload_remove_openingd(struct subd *openingd,
payload->openingd = NULL; payload->openingd = NULL;
} }
static void openchannel_hook_cb(struct openchannel_hook_payload *payload, static void openchannel_hook_cb(struct openchannel_hook_payload *payload STEALS,
const char *buffer, const char *buffer,
const jsmntok_t *toks) const jsmntok_t *toks)
{ {
@ -829,7 +829,6 @@ static void openchannel_hook_cb(struct openchannel_hook_payload *payload,
REGISTER_PLUGIN_HOOK(openchannel, REGISTER_PLUGIN_HOOK(openchannel,
PLUGIN_HOOK_SINGLE, PLUGIN_HOOK_SINGLE,
openchannel_hook_cb, openchannel_hook_cb,
struct openchannel_hook_payload *,
openchannel_hook_serialize, openchannel_hook_serialize,
struct openchannel_hook_payload *); struct openchannel_hook_payload *);
@ -847,7 +846,7 @@ static void opening_got_offer(struct subd *openingd,
return; return;
} }
payload = tal(openingd->ld, struct openchannel_hook_payload); payload = tal(openingd, struct openchannel_hook_payload);
payload->openingd = openingd; payload->openingd = openingd;
if (!fromwire_opening_got_offer(payload, msg, if (!fromwire_opening_got_offer(payload, msg,
&payload->funding_satoshis, &payload->funding_satoshis,
@ -868,7 +867,7 @@ static void opening_got_offer(struct subd *openingd,
} }
tal_add_destructor2(openingd, openchannel_payload_remove_openingd, payload); tal_add_destructor2(openingd, openchannel_payload_remove_openingd, payload);
plugin_hook_call_openchannel(openingd->ld, payload, payload); plugin_hook_call_openchannel(openingd->ld, payload);
} }
static unsigned int openingd_msg(struct subd *openingd, static unsigned int openingd_msg(struct subd *openingd,

View File

@ -860,7 +860,7 @@ peer_connected_serialize(struct peer_connected_hook_payload *payload,
} }
static void static void
peer_connected_hook_cb(struct peer_connected_hook_payload *payload, peer_connected_hook_cb(struct peer_connected_hook_payload *payload STEALS,
const char *buffer, const char *buffer,
const jsmntok_t *toks) const jsmntok_t *toks)
{ {
@ -974,7 +974,6 @@ send_error:
REGISTER_PLUGIN_HOOK(peer_connected, PLUGIN_HOOK_SINGLE, REGISTER_PLUGIN_HOOK(peer_connected, PLUGIN_HOOK_SINGLE,
peer_connected_hook_cb, peer_connected_hook_cb,
struct peer_connected_hook_payload *,
peer_connected_serialize, peer_connected_serialize,
struct peer_connected_hook_payload *); struct peer_connected_hook_payload *);
@ -1018,7 +1017,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg,
assert(!peer->uncommitted_channel); assert(!peer->uncommitted_channel);
hook_payload->channel = peer_active_channel(peer); hook_payload->channel = peer_active_channel(peer);
plugin_hook_call_peer_connected(ld, hook_payload, hook_payload); plugin_hook_call_peer_connected(ld, hook_payload);
} }
/* FIXME: Unify our watch code so we get notified by txout, instead, like /* FIXME: Unify our watch code so we get notified by txout, instead, like
@ -2297,7 +2296,7 @@ struct custommsg_payload {
const u8 *msg; const u8 *msg;
}; };
static void custommsg_callback(struct custommsg_payload *payload, static void custommsg_callback(struct custommsg_payload *payload STEALS,
const char *buffer, const jsmntok_t *toks) const char *buffer, const jsmntok_t *toks)
{ {
tal_free(payload); tal_free(payload);
@ -2310,8 +2309,9 @@ static void custommsg_payload_serialize(struct custommsg_payload *payload,
json_add_node_id(stream, "peer_id", &payload->peer_id); json_add_node_id(stream, "peer_id", &payload->peer_id);
} }
REGISTER_PLUGIN_HOOK(custommsg, PLUGIN_HOOK_SINGLE, custommsg_callback, REGISTER_PLUGIN_HOOK(custommsg, PLUGIN_HOOK_SINGLE,
struct custommsg_payload *, custommsg_payload_serialize, custommsg_callback,
custommsg_payload_serialize,
struct custommsg_payload *); struct custommsg_payload *);
void handle_custommsg_in(struct lightningd *ld, const struct node_id *peer_id, void handle_custommsg_in(struct lightningd *ld, const struct node_id *peer_id,
@ -2329,7 +2329,7 @@ void handle_custommsg_in(struct lightningd *ld, const struct node_id *peer_id,
p->peer_id = *peer_id; p->peer_id = *peer_id;
p->msg = tal_steal(p, custommsg); p->msg = tal_steal(p, custommsg);
plugin_hook_call_custommsg(ld, p, p); plugin_hook_call_custommsg(ld, p);
} }
static struct command_result *json_sendcustommsg(struct command *cmd, static struct command_result *json_sendcustommsg(struct command *cmd,

View File

@ -976,7 +976,7 @@ static void htlc_accepted_hook_serialize(struct htlc_accepted_hook_payload *p,
* Callback when a plugin answers to the htlc_accepted hook * Callback when a plugin answers to the htlc_accepted hook
*/ */
static void static void
htlc_accepted_hook_callback(struct htlc_accepted_hook_payload *request, htlc_accepted_hook_callback(struct htlc_accepted_hook_payload *request STEALS,
const char *buffer, const jsmntok_t *toks) const char *buffer, const jsmntok_t *toks)
{ {
struct route_step *rs = request->route_step; struct route_step *rs = request->route_step;
@ -1027,7 +1027,6 @@ htlc_accepted_hook_callback(struct htlc_accepted_hook_payload *request,
REGISTER_PLUGIN_HOOK(htlc_accepted, PLUGIN_HOOK_CHAIN, REGISTER_PLUGIN_HOOK(htlc_accepted, PLUGIN_HOOK_CHAIN,
htlc_accepted_hook_callback, htlc_accepted_hook_callback,
struct htlc_accepted_hook_payload *,
htlc_accepted_hook_serialize, htlc_accepted_hook_serialize,
struct htlc_accepted_hook_payload *); struct htlc_accepted_hook_payload *);
@ -1160,7 +1159,7 @@ static bool peer_accepted_htlc(const tal_t *ctx,
goto fail; goto fail;
} }
hook_payload = tal(hin, struct htlc_accepted_hook_payload); hook_payload = tal(NULL, struct htlc_accepted_hook_payload);
hook_payload->route_step = tal_steal(hook_payload, rs); hook_payload->route_step = tal_steal(hook_payload, rs);
hook_payload->payload = onion_decode(hook_payload, rs, hook_payload->payload = onion_decode(hook_payload, rs,
@ -1186,7 +1185,7 @@ static bool peer_accepted_htlc(const tal_t *ctx,
#endif #endif
hook_payload->next_blinding = NULL; hook_payload->next_blinding = NULL;
plugin_hook_call_htlc_accepted(ld, hook_payload, hook_payload); plugin_hook_call_htlc_accepted(ld, hook_payload);
/* Falling through here is ok, after all the HTLC locked */ /* Falling through here is ok, after all the HTLC locked */
return true; return true;

View File

@ -14,7 +14,6 @@ struct plugin_hook_request {
struct plugin *plugin; struct plugin *plugin;
const struct plugin_hook *hook; const struct plugin_hook *hook;
void *cb_arg; void *cb_arg;
void *payload;
struct db *db; struct db *db;
struct lightningd *ld; struct lightningd *ld;
}; };
@ -221,13 +220,13 @@ static void plugin_hook_call_next(struct plugin_hook_request *ph_req)
plugin_get_log(ph_req->plugin), plugin_get_log(ph_req->plugin),
plugin_hook_callback, ph_req); plugin_hook_callback, ph_req);
hook->serialize_payload(ph_req->payload, req->stream); hook->serialize_payload(ph_req->cb_arg, req->stream);
jsonrpc_request_end(req); jsonrpc_request_end(req);
plugin_request_send(ph_req->plugin, req); plugin_request_send(ph_req->plugin, req);
} }
void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook, void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
void *payload, void *cb_arg) tal_t *cb_arg STEALS)
{ {
struct plugin_hook_request *ph_req; struct plugin_hook_request *ph_req;
struct plugin_hook_call_link *link; struct plugin_hook_call_link *link;
@ -239,9 +238,8 @@ void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
* to eventually to inspect in-flight requests. */ * to eventually to inspect in-flight requests. */
ph_req = notleak(tal(hook->plugins, struct plugin_hook_request)); ph_req = notleak(tal(hook->plugins, struct plugin_hook_request));
ph_req->hook = hook; ph_req->hook = hook;
ph_req->cb_arg = cb_arg; ph_req->cb_arg = tal_steal(ph_req, cb_arg);
ph_req->db = ld->wallet->db; ph_req->db = ld->wallet->db;
ph_req->payload = tal_steal(ph_req, payload);
ph_req->ld = ld; ph_req->ld = ld;
list_head_init(&ph_req->call_chain); list_head_init(&ph_req->call_chain);

View File

@ -28,13 +28,14 @@
* delivery to the plugin, and from a JSON-object to the internal * delivery to the plugin, and from a JSON-object to the internal
* representation: * representation:
* *
* - `serialize_payload` which takes a payload of type `payload_type` * - `serialize_payload` which takes a payload of type `cb_arg_type`
* and serializes it into the given `json_stream`. ` * and serializes it into the given `json_stream`. `
* *
* - `response_cb` is called once the plugin has responded (or with * - `response_cb` is called once the plugin has responded (or with
* buffer == NULL if there's no plugin). In addition an arbitrary * buffer == NULL if there's no plugin). In addition an arbitrary
* additional argument of type `cb_arg_type` can be passed along * additional argument of type `cb_arg_type` can be passed along
* that may contain any additional context necessary. * that may contain any additional context necessary. It must free
* or otherwise take ownership of the cb_arg_type argument.
* *
* *
* To make hook invocations easier, each hook registered with * To make hook invocations easier, each hook registered with
@ -69,7 +70,7 @@ AUTODATA_TYPE(hooks, struct plugin_hook);
* wrappers generated by the `PLUGIN_HOOK_REGISTER` macro. * wrappers generated by the `PLUGIN_HOOK_REGISTER` macro.
*/ */
void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook, void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
void *payload, void *cb_arg); tal_t *cb_arg STEALS);
/* Create a small facade in from of `plugin_hook_call_` to make sure /* Create a small facade in from of `plugin_hook_call_` to make sure
@ -78,13 +79,11 @@ void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
* the method-name is correct for the call. * the method-name is correct for the call.
*/ */
/* FIXME: Find a way to avoid back-to-back declaration and definition */ /* FIXME: Find a way to avoid back-to-back declaration and definition */
#define PLUGIN_HOOK_CALL_DEF(name, payload_type, response_cb_arg_type) \ #define PLUGIN_HOOK_CALL_DEF(name, cb_arg_type) \
UNNEEDED static inline void plugin_hook_call_##name( \ UNNEEDED static inline void plugin_hook_call_##name( \
struct lightningd *ld, payload_type payload, \ struct lightningd *ld, cb_arg_type cb_arg STEALS) \
response_cb_arg_type cb_arg) \
{ \ { \
plugin_hook_call_(ld, &name##_hook_gen, (void *)payload, \ plugin_hook_call_(ld, &name##_hook_gen, cb_arg); \
(void *)cb_arg); \
} }
/* Typechecked registration of a plugin hook. We check that the /* Typechecked registration of a plugin hook. We check that the
@ -95,23 +94,22 @@ void plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
* response_cb function accepts the deserialized response format and * response_cb function accepts the deserialized response format and
* an arbitrary extra argument used to maintain context. * an arbitrary extra argument used to maintain context.
*/ */
#define REGISTER_PLUGIN_HOOK(name, type, response_cb, response_cb_arg_type, \ #define REGISTER_PLUGIN_HOOK(name, type, response_cb, \
serialize_payload, payload_type) \ serialize_payload, cb_arg_type) \
struct plugin_hook name##_hook_gen = { \ struct plugin_hook name##_hook_gen = { \
stringify(name), \ stringify(name), \
type, \ type, \
typesafe_cb_cast( \ typesafe_cb_cast( \
void (*)(void *, const char *, const jsmntok_t *), \ void (*)(void *STEALS, const char *, const jsmntok_t *), \
void (*)(response_cb_arg_type, const char *, \ void (*)(cb_arg_type STEALS, const char *, const jsmntok_t *), \
const jsmntok_t *), \
response_cb), \ response_cb), \
typesafe_cb_cast(void (*)(void *, struct json_stream *), \ typesafe_cb_cast(void (*)(void *, struct json_stream *), \
void (*)(payload_type, struct json_stream *), \ void (*)(cb_arg_type, struct json_stream *), \
serialize_payload), \ serialize_payload), \
NULL, /* .plugins */ \ NULL, /* .plugins */ \
}; \ }; \
AUTODATA(hooks, &name##_hook_gen); \ AUTODATA(hooks, &name##_hook_gen); \
PLUGIN_HOOK_CALL_DEF(name, payload_type, response_cb_arg_type); PLUGIN_HOOK_CALL_DEF(name, cb_arg_type)
bool plugin_hook_register(struct plugin *plugin, const char *method); bool plugin_hook_register(struct plugin *plugin, const char *method);

View File

@ -375,7 +375,7 @@ void per_peer_state_set_fds(struct per_peer_state *pps UNNEEDED,
{ fprintf(stderr, "per_peer_state_set_fds called!\n"); abort(); } { fprintf(stderr, "per_peer_state_set_fds called!\n"); abort(); }
/* Generated stub for plugin_hook_call_ */ /* Generated stub for plugin_hook_call_ */
void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED, void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED,
void *payload UNNEEDED, void *cb_arg UNNEEDED) tal_t *cb_arg UNNEEDED)
{ fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); } { fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); }
/* Generated stub for subd_release_channel */ /* Generated stub for subd_release_channel */
void subd_release_channel(struct subd *owner UNNEEDED, void *channel UNNEEDED) void subd_release_channel(struct subd *owner UNNEEDED, void *channel UNNEEDED)

View File

@ -96,7 +96,7 @@ struct command_result *param_tok(struct command *cmd UNNEEDED, const char *name
{ fprintf(stderr, "param_tok called!\n"); abort(); } { fprintf(stderr, "param_tok called!\n"); abort(); }
/* Generated stub for plugin_hook_call_ */ /* Generated stub for plugin_hook_call_ */
void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED, void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED,
void *payload UNNEEDED, void *cb_arg UNNEEDED) tal_t *cb_arg UNNEEDED)
{ fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); } { fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); }
/* AUTOGENERATED MOCKS END */ /* AUTOGENERATED MOCKS END */

View File

@ -547,7 +547,7 @@ void per_peer_state_set_fds(struct per_peer_state *pps UNNEEDED,
{ fprintf(stderr, "per_peer_state_set_fds called!\n"); abort(); } { fprintf(stderr, "per_peer_state_set_fds called!\n"); abort(); }
/* Generated stub for plugin_hook_call_ */ /* Generated stub for plugin_hook_call_ */
void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED, void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED,
void *payload UNNEEDED, void *cb_arg UNNEEDED) tal_t *cb_arg UNNEEDED)
{ fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); } { fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); }
/* Generated stub for process_onionpacket */ /* Generated stub for process_onionpacket */
struct route_step *process_onionpacket( struct route_step *process_onionpacket(