diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 910533362..60b8474eb 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -404,10 +404,7 @@ static void sendrawtx_callback(const char *buf, const jsmntok_t *toks, err); } - db_begin_transaction(call->bitcoind->ld->wallet->db); call->cb(call->bitcoind, success, errmsg, call->cb_arg); - db_commit_transaction(call->bitcoind->ld->wallet->db); - tal_free(call); } @@ -472,9 +469,7 @@ getrawblockbyheight_callback(const char *buf, const jsmntok_t *toks, * with NULL values. */ err = json_scan(tmpctx, buf, toks, "{result:{blockhash:null}}"); if (!err) { - db_begin_transaction(call->bitcoind->ld->wallet->db); call->cb(call->bitcoind, NULL, NULL, call->cb_arg); - db_commit_transaction(call->bitcoind->ld->wallet->db); goto clean; } @@ -493,9 +488,7 @@ getrawblockbyheight_callback(const char *buf, const jsmntok_t *toks, "getrawblockbyheight", "bad block"); - db_begin_transaction(call->bitcoind->ld->wallet->db); call->cb(call->bitcoind, &blkid, blk, call->cb_arg); - db_commit_transaction(call->bitcoind->ld->wallet->db); clean: tal_free(call); @@ -574,10 +567,8 @@ static void getchaininfo_callback(const char *buf, const jsmntok_t *toks, bitcoin_plugin_error(call->bitcoind, buf, toks, "getchaininfo", "bad 'result' field: %s", err); - db_begin_transaction(call->bitcoind->ld->wallet->db); call->cb(call->bitcoind, chain, headers, blocks, ibd, call->first_call, call->cb_arg); - db_commit_transaction(call->bitcoind->ld->wallet->db); tal_free(call); } @@ -640,9 +631,7 @@ static void getutxout_callback(const char *buf, const jsmntok_t *toks, err = json_scan(tmpctx, buf, toks, "{result:{script:null}}"); if (!err) { - db_begin_transaction(call->bitcoind->ld->wallet->db); call->cb(call->bitcoind, NULL, call->cb_arg); - db_commit_transaction(call->bitcoind->ld->wallet->db); goto clean; } @@ -654,9 +643,7 @@ static void getutxout_callback(const char *buf, const jsmntok_t *toks, bitcoin_plugin_error(call->bitcoind, buf, toks, "getutxout", "bad 'result' field: %s", err); - db_begin_transaction(call->bitcoind->ld->wallet->db); call->cb(call->bitcoind, &txout, call->cb_arg); - db_commit_transaction(call->bitcoind->ld->wallet->db); clean: tal_free(call); diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 2520c0eac..89a265f00 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -921,7 +921,6 @@ static void listincoming_done(const char *buffer, const jsmntok_t *idtok UNUSED, struct invoice_info *info) { - struct lightningd *ld = info->cmd->ld; struct command_result *ret; bool warning_mpp, warning_capacity, warning_deadends, warning_offline, warning_private_unused; @@ -934,8 +933,6 @@ static void listincoming_done(const char *buffer, if (ret) return; - /* We're actually outside a db transaction here: spooky! */ - db_begin_transaction(ld->wallet->db); invoice_complete(info, false, warning_mpp, @@ -943,7 +940,6 @@ static void listincoming_done(const char *buffer, warning_deadends, warning_offline, warning_private_unused); - db_commit_transaction(ld->wallet->db); } /* Since this is a dev-only option, we will crash if dev-routes is not diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 8042ea1ba..8ca6b0369 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -84,6 +85,7 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, p->blacklist = tal_arr(p, const char *, 0); p->plugin_idx = 0; p->dev_builtin_plugins_unimportant = false; + p->want_db_transaction = true; return p; } @@ -619,6 +621,7 @@ static const char *plugin_response_handle(struct plugin *plugin, /* We expect the request->cb to copy if needed */ pd = plugin_detect_destruction(plugin); + request->response_cb(plugin->buffer, toks, idtok, request->response_cb_arg); /* Note that in the case of 'plugin stop' this can free request (since @@ -639,12 +642,14 @@ static const char *plugin_response_handle(struct plugin *plugin, * If @destroyed was set, it means the plugin called plugin stop on itself. */ static const char *plugin_read_json_one(struct plugin *plugin, + bool want_transaction, bool *complete, bool *destroyed) { const jsmntok_t *jrtok, *idtok; struct plugin_destroyed *pd; const char *err; + struct wallet *wallet = plugin->plugins->ld->wallet; *destroyed = false; /* Note that in the case of 'plugin stop' this can free request (since @@ -687,6 +692,11 @@ static const char *plugin_read_json_one(struct plugin *plugin, "JSON-RPC message does not contain \"jsonrpc\" field"); } + /* We can be called extremely early, or as db hook, or for + * fake "terminated" request. */ + if (want_transaction) + db_begin_transaction(wallet->db); + pd = plugin_detect_destruction(plugin); if (!idtok) { /* A Notification is a Request object without an "id" @@ -732,6 +742,8 @@ static const char *plugin_read_json_one(struct plugin *plugin, */ err = plugin_response_handle(plugin, plugin->toks, idtok); } + if (want_transaction) + db_commit_transaction(wallet->db); /* Corner case: rpc_command hook can destroy plugin for 'plugin * stop'! */ @@ -753,6 +765,9 @@ static struct io_plan *plugin_read_json(struct io_conn *conn, { bool success; bool have_full; + /* wallet is NULL in really early code */ + bool want_transaction = (plugin->plugins->want_db_transaction + && plugin->plugins->ld->wallet != NULL); log_io(plugin->log, LOG_IO_IN, NULL, "", plugin->buffer + plugin->used, plugin->len_read); @@ -774,8 +789,9 @@ static struct io_plan *plugin_read_json(struct io_conn *conn, do { bool destroyed; const char *err; - err = - plugin_read_json_one(plugin, &success, &destroyed); + + err = plugin_read_json_one(plugin, want_transaction, + &success, &destroyed); /* If it's destroyed, conn is already freed! */ if (destroyed) diff --git a/lightningd/plugin.h b/lightningd/plugin.h index a7298d7d4..025abdda3 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -108,6 +108,10 @@ struct plugins { struct list_head plugins; bool startup; + /* Normally we want to wrap callbacks in a db transaction, but + * not for the db hook servicing */ + bool want_db_transaction; + struct logger *log; struct lightningd *ld; diff --git a/lightningd/plugin_hook.c b/lightningd/plugin_hook.c index fe392f486..6793fbbf4 100644 --- a/lightningd/plugin_hook.c +++ b/lightningd/plugin_hook.c @@ -134,7 +134,10 @@ static void plugin_hook_killed(struct plugin_hook_call_link *link) /* Call next will unlink, so we don't need to. This is treated * equivalent to the plugin returning a continue-result. */ + struct db *db = link->plugin->plugins->ld->wallet->db; + db_begin_transaction(db); plugin_hook_callback(NULL, NULL, NULL, link->req); + db_commit_transaction(db); } else { /* The plugin is in the list waiting to be called, just remove * it from the list. */ @@ -159,9 +162,7 @@ static void plugin_hook_callback(const char *buffer, const jsmntok_t *toks, struct plugin_hook_request *r) { const jsmntok_t *resulttok; - struct db *db = r->db; struct plugin_hook_call_link *last, *it; - bool in_transaction = false; /* Pop the head off the call chain and continue with the next */ last = list_pop(&r->call_chain, struct plugin_hook_call_link, list); @@ -187,31 +188,22 @@ static void plugin_hook_callback(const char *buffer, const jsmntok_t *toks, r->hook->name, toks->end - toks->start, buffer + toks->start); - db_begin_transaction(db); if (!r->hook->deserialize_cb(r->cb_arg, buffer, resulttok)) { tal_free(r->cb_arg); - db_commit_transaction(db); goto cleanup; } - in_transaction = true; } else { /* plugin died */ resulttok = NULL; } if (!list_empty(&r->call_chain)) { - if (in_transaction) - db_commit_transaction(db); plugin_hook_call_next(r); return; } - /* We optimize for the case where we already called deserialize_cb */ - if (!in_transaction) - db_begin_transaction(db); r->hook->final_cb(r->cb_arg); - db_commit_transaction(db); cleanup: /* We need to remove the destructors from the remaining @@ -349,7 +341,8 @@ void plugin_hook_db_sync(struct db *db) struct jsonrpc_request *req; struct plugin_hook_request *ph_req; void *ret; - struct plugin **plugins; + struct plugin **plugin_arr; + struct plugins *plugins; size_t i; size_t num_hooks; @@ -358,11 +351,12 @@ void plugin_hook_db_sync(struct db *db) if (num_hooks == 0) return; - plugins = notleak(tal_arr(NULL, struct plugin *, + plugin_arr = notleak(tal_arr(NULL, struct plugin *, num_hooks)); for (i = 0; i < num_hooks; ++i) - plugins[i] = hook->hooks[i]->plugin; + plugin_arr[i] = hook->hooks[i]->plugin; + plugins = plugin_arr[0]->plugins; ph_req = notleak(tal(hook->hooks, struct plugin_hook_request)); ph_req->hook = hook; ph_req->db = db; @@ -372,7 +366,7 @@ void plugin_hook_db_sync(struct db *db) /* Create an object for this plugin. */ struct db_write_hook_req *dwh_req; dwh_req = tal(ph_req, struct db_write_hook_req); - dwh_req->plugin = plugins[i]; + dwh_req->plugin = plugin_arr[i]; dwh_req->ph_req = ph_req; dwh_req->num_hooks = &num_hooks; @@ -393,21 +387,25 @@ void plugin_hook_db_sync(struct db *db) json_array_end(req->stream); jsonrpc_request_end(req); - plugin_request_send(plugins[i], req); + plugin_request_send(plugin_arr[i], req); } + /* We don't want to try to open another transaction: we're in one! */ + plugins->want_db_transaction = false; + ret = plugins_exclusive_loop(plugin_arr); + plugins->want_db_transaction = true; + /* We can be called on way out of an io_loop, which is already breaking. * That will make this immediately return; save the break value and call * again, then hand it onwards. */ - ret = plugins_exclusive_loop(plugins); if (ret != ph_req) { - void *ret2 = plugins_exclusive_loop(plugins); + void *ret2 = plugins_exclusive_loop(plugin_arr); assert(ret2 == ph_req); - log_debug(plugins[0]->plugins->ld->log, "io_break: %s", __func__); + log_debug(plugins->ld->log, "io_break: %s", __func__); io_break(ret); } assert(num_hooks == 0); - tal_free(plugins); + tal_free(plugin_arr); tal_free(ph_req); }