diff --git a/src/httprpc.cpp b/src/httprpc.cpp index afdb6784aef..947f1d8a1fa 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -32,45 +32,6 @@ using util::TrimStringView; /** WWW-Authenticate to present with 401 Unauthorized response */ static const char* WWW_AUTH_HEADER_DATA = "Basic realm=\"jsonrpc\""; -/** Simple one-shot callback timer to be used by the RPC mechanism to e.g. - * re-lock the wallet. - */ -class HTTPRPCTimer : public RPCTimerBase -{ -public: - HTTPRPCTimer(struct event_base* eventBase, std::function& func, int64_t millis) : - ev(eventBase, false, func) - { - struct timeval tv; - tv.tv_sec = millis/1000; - tv.tv_usec = (millis%1000)*1000; - ev.trigger(&tv); - } -private: - HTTPEvent ev; -}; - -class HTTPRPCTimerInterface : public RPCTimerInterface -{ -public: - explicit HTTPRPCTimerInterface(struct event_base* _base) : base(_base) - { - } - const char* Name() override - { - return "HTTP"; - } - RPCTimerBase* NewTimer(std::function& func, int64_t millis) override - { - return new HTTPRPCTimer(base, func, millis); - } -private: - struct event_base* base; -}; - - -/* Stored RPC timer interface (for unregistration) */ -static std::unique_ptr httpRPCTimerInterface; /* List of -rpcauth values */ static std::vector> g_rpcauth; /* RPC Auth Whitelist */ @@ -380,8 +341,6 @@ bool StartHTTPRPC(const std::any& context) } struct event_base* eventBase = EventBase(); assert(eventBase); - httpRPCTimerInterface = std::make_unique(eventBase); - RPCSetTimerInterface(httpRPCTimerInterface.get()); return true; } @@ -397,8 +356,4 @@ void StopHTTPRPC() if (g_wallet_init_interface.HasWalletSupport()) { UnregisterHTTPHandler("/wallet/", false); } - if (httpRPCTimerInterface) { - RPCUnsetTimerInterface(httpRPCTimerInterface.get()); - httpRPCTimerInterface.reset(); - } } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index f6e831624d0..239106afedc 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -352,9 +352,6 @@ public: //! Check if deprecated RPC is enabled. virtual bool rpcEnableDeprecated(const std::string& method) = 0; - //! Run function after given number of seconds. Cancel any previous calls with same name. - virtual void rpcRunLater(const std::string& name, std::function fn, int64_t seconds) = 0; - //! Get settings value. virtual common::SettingsValue getSetting(const std::string& arg) = 0; diff --git a/src/interfaces/node.h b/src/interfaces/node.h index a74695be85e..78a186c5d96 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -27,7 +27,6 @@ class BanMan; class CFeeRate; class CNodeStats; class Coin; -class RPCTimerInterface; class UniValue; class Proxy; enum class SynchronizationState; @@ -205,12 +204,6 @@ public: //! List rpc commands. virtual std::vector listRpcCommands() = 0; - //! Set RPC timer interface if unset. - virtual void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) = 0; - - //! Unset RPC timer interface. - virtual void rpcUnsetTimerInterface(RPCTimerInterface* iface) = 0; - //! Get unspent output associated with a transaction. virtual std::optional getUnspentOutput(const COutPoint& output) = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 59bc9fed2d4..0485ff48bd1 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -356,8 +356,6 @@ public: return ::tableRPC.execute(req); } std::vector listRpcCommands() override { return ::tableRPC.listCommands(); } - void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) override { RPCSetTimerInterfaceIfUnset(iface); } - void rpcUnsetTimerInterface(RPCTimerInterface* iface) override { RPCUnsetTimerInterface(iface); } std::optional getUnspentOutput(const COutPoint& output) override { LOCK(::cs_main); @@ -804,10 +802,6 @@ public: return std::make_unique(command); } bool rpcEnableDeprecated(const std::string& method) override { return IsDeprecatedRPCEnabled(method); } - void rpcRunLater(const std::string& name, std::function fn, int64_t seconds) override - { - RPCRunLater(name, std::move(fn), seconds); - } common::SettingsValue getSetting(const std::string& name) override { return args().GetSetting(name); diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 541b7124f58..958c82fda22 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -98,37 +98,6 @@ private: interfaces::Node& m_node; }; -/** Class for handling RPC timers - * (used for e.g. re-locking the wallet after a timeout) - */ -class QtRPCTimerBase: public QObject, public RPCTimerBase -{ - Q_OBJECT -public: - QtRPCTimerBase(std::function& _func, int64_t millis): - func(_func) - { - timer.setSingleShot(true); - connect(&timer, &QTimer::timeout, [this]{ func(); }); - timer.start(millis); - } - ~QtRPCTimerBase() = default; -private: - QTimer timer; - std::function func; -}; - -class QtRPCTimerInterface: public RPCTimerInterface -{ -public: - ~QtRPCTimerInterface() = default; - const char *Name() override { return "Qt"; } - RPCTimerBase* NewTimer(std::function& func, int64_t millis) override - { - return new QtRPCTimerBase(func, millis); - } -}; - class PeerIdViewDelegate : public QStyledItemDelegate { Q_OBJECT @@ -567,12 +536,6 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty ui->WalletSelector->setVisible(false); ui->WalletSelectorLabel->setVisible(false); - // Register RPC timer interface - rpcTimerInterface = new QtRPCTimerInterface(); - // avoid accidentally overwriting an existing, non QTThread - // based timer interface - m_node.rpcSetTimerInterfaceIfUnset(rpcTimerInterface); - setTrafficGraphRange(INITIAL_TRAFFIC_GRAPH_MINS); updateDetailWidget(); @@ -602,8 +565,6 @@ RPCConsole::~RPCConsole() settings.setValue("PeersTabPeerHeaderState", m_peer_widget_header_state); settings.setValue("PeersTabBanlistHeaderState", m_banlist_widget_header_state); - m_node.rpcUnsetTimerInterface(rpcTimerInterface); - delete rpcTimerInterface; delete ui; } diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index 67922ceef2a..a1b9522b98e 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -20,7 +20,6 @@ class PlatformStyle; class RPCExecutor; -class RPCTimerInterface; class WalletModel; namespace interfaces { @@ -166,7 +165,6 @@ private: QString cmdBeforeBrowsing; QList cachedNodeids; const PlatformStyle* const platformStyle; - RPCTimerInterface *rpcTimerInterface = nullptr; QMenu *peersTableContextMenu = nullptr; QMenu *banTableContextMenu = nullptr; int consoleFontSize = 0; diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index bd99615bbf0..ef284de15b4 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -34,11 +34,6 @@ static GlobalMutex g_rpc_warmup_mutex; static std::atomic g_rpc_running{false}; static bool fRPCInWarmup GUARDED_BY(g_rpc_warmup_mutex) = true; static std::string rpcWarmupStatus GUARDED_BY(g_rpc_warmup_mutex) = "RPC server started"; -/* Timer-creating functions */ -static RPCTimerInterface* timerInterface = nullptr; -/* Map of name to timer. */ -static GlobalMutex g_deadline_timers_mutex; -static std::map > deadlineTimers GUARDED_BY(g_deadline_timers_mutex); static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler); struct RPCCommandExecutionInfo @@ -301,7 +296,6 @@ void StopRPC() assert(!g_rpc_running); std::call_once(g_rpc_stop_flag, [&]() { LogDebug(BCLog::RPC, "Stopping RPC\n"); - WITH_LOCK(g_deadline_timers_mutex, deadlineTimers.clear()); DeleteAuthCookie(); LogDebug(BCLog::RPC, "RPC stopped.\n"); }); @@ -543,31 +537,4 @@ UniValue CRPCTable::dumpArgMap(const JSONRPCRequest& args_request) const return ret; } -void RPCSetTimerInterfaceIfUnset(RPCTimerInterface *iface) -{ - if (!timerInterface) - timerInterface = iface; -} - -void RPCSetTimerInterface(RPCTimerInterface *iface) -{ - timerInterface = iface; -} - -void RPCUnsetTimerInterface(RPCTimerInterface *iface) -{ - if (timerInterface == iface) - timerInterface = nullptr; -} - -void RPCRunLater(const std::string& name, std::function func, int64_t nSeconds) -{ - if (!timerInterface) - throw JSONRPCError(RPC_INTERNAL_ERROR, "No timer handler registered for RPC"); - LOCK(g_deadline_timers_mutex); - deadlineTimers.erase(name); - LogDebug(BCLog::RPC, "queue run of timer %s in %i seconds (using %s)\n", name, nSeconds, timerInterface->Name()); - deadlineTimers.emplace(name, std::unique_ptr(timerInterface->NewTimer(func, nSeconds*1000))); -} - CRPCTable tableRPC; diff --git a/src/rpc/server.h b/src/rpc/server.h index d4b48f2418f..2c17048dbe5 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -35,47 +35,6 @@ void SetRPCWarmupFinished(); /* returns the current warmup state. */ bool RPCIsInWarmup(std::string *outStatus); -/** Opaque base class for timers returned by NewTimerFunc. - * This provides no methods at the moment, but makes sure that delete - * cleans up the whole state. - */ -class RPCTimerBase -{ -public: - virtual ~RPCTimerBase() = default; -}; - -/** - * RPC timer "driver". - */ -class RPCTimerInterface -{ -public: - virtual ~RPCTimerInterface() = default; - /** Implementation name */ - virtual const char *Name() = 0; - /** Factory function for timers. - * RPC will call the function to create a timer that will call func in *millis* milliseconds. - * @note As the RPC mechanism is backend-neutral, it can use different implementations of timers. - * This is needed to cope with the case in which there is no HTTP server, but - * only GUI RPC console, and to break the dependency of pcserver on httprpc. - */ - virtual RPCTimerBase* NewTimer(std::function& func, int64_t millis) = 0; -}; - -/** Set the factory function for timers */ -void RPCSetTimerInterface(RPCTimerInterface *iface); -/** Set the factory function for timer, but only, if unset */ -void RPCSetTimerInterfaceIfUnset(RPCTimerInterface *iface); -/** Unset factory function for timers */ -void RPCUnsetTimerInterface(RPCTimerInterface *iface); - -/** - * Run func nSeconds from now. - * Overrides previous timer (if any). - */ -void RPCRunLater(const std::string& name, std::function func, int64_t nSeconds); - typedef RPCHelpMan (*RpcMethodFnType)(); class CRPCCommand diff --git a/src/wallet/rpc/encrypt.cpp b/src/wallet/rpc/encrypt.cpp index 78637085738..a5f1cea91ee 100644 --- a/src/wallet/rpc/encrypt.cpp +++ b/src/wallet/rpc/encrypt.cpp @@ -3,6 +3,8 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include +#include #include #include @@ -88,24 +90,24 @@ RPCHelpMan walletpassphrase() relock_time = pwallet->nRelockTime; } - // rpcRunLater must be called without cs_wallet held otherwise a deadlock - // can occur. The deadlock would happen when RPCRunLater removes the - // previous timer (and waits for the callback to finish if already running) - // and the callback locks cs_wallet. - AssertLockNotHeld(wallet->cs_wallet); + // Get wallet scheduler to queue up the relock callback in the future. + // Scheduled events don't get destructed until they are executed, + // and they are executed in series in a single scheduler thread so + // no cs_wallet lock is needed. + WalletContext& context = EnsureWalletContext(request.context); // Keep a weak pointer to the wallet so that it is possible to unload the // wallet before the following callback is called. If a valid shared pointer // is acquired in the callback then the wallet is still loaded. std::weak_ptr weak_wallet = wallet; - pwallet->chain().rpcRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), [weak_wallet, relock_time] { + context.scheduler->scheduleFromNow([weak_wallet, relock_time] { if (auto shared_wallet = weak_wallet.lock()) { LOCK2(shared_wallet->m_relock_mutex, shared_wallet->cs_wallet); - // Skip if this is not the most recent rpcRunLater callback. + // Skip if this is not the most recent relock callback. if (shared_wallet->nRelockTime != relock_time) return; shared_wallet->Lock(); shared_wallet->nRelockTime = 0; } - }, nSleepTime); + }, std::chrono::seconds(nSleepTime)); return UniValue::VNULL; }, diff --git a/test/functional/wallet_keypool.py b/test/functional/wallet_keypool.py index 78c0426e3a8..03fed4dfbf2 100755 --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the wallet keypool and interaction with wallet encryption/locking.""" -import time from decimal import Decimal from test_framework.test_framework import BitcoinTestFramework @@ -127,8 +126,10 @@ class KeyPoolTest(BitcoinTestFramework): nodes[0].keypoolrefill(3) # test walletpassphrase timeout - time.sleep(1.1) - assert_equal(nodes[0].getwalletinfo()["unlocked_until"], 0) + # CScheduler relies on condition_variable::wait_until() which does not + # guarantee accurate timing. We'll wait up to 5 seconds to execute a 1 + # second scheduled event. + nodes[0].wait_until(lambda: nodes[0].getwalletinfo()["unlocked_until"] == 0, timeout=5) # drain the keypool for _ in range(3):