From c919d4940a03c1c3ed91f7b47b4f56a2c8997d9e Mon Sep 17 00:00:00 2001 From: SomberNight Date: Tue, 18 Nov 2025 16:21:49 +0000 Subject: [PATCH] keystore: fix memory leak for LRU cache Using `@functools.lru_cache` on an instance method behaves in interesting ways. The cache kept a ref around for `self`, so in effect we were never GC-ing keystore objects. Effectively there was a single global cache for derive_pubkey, with keys `(keystore, for_change, n)`. This PR now changes the caching to be per-keystore: each ks has a cache, keyed `(for_change, n)`. GC-ing individual keystores should now be possible, which should result in cleaning up just their own cache. This also enables the corresponding previously silence flake8-bugbear check for `@functools.lru_cache`. (note that the check can selectively be disabled by adding a comment on the relevant line: `# noqa: B019`) --- .cirrus.yml | 2 +- electrum/keystore.py | 24 +++++++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index e7c68d672..fdc6cf441 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -225,7 +225,7 @@ task: # - https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes # - https://github.com/PyCQA/flake8-bugbear/tree/8c0e7eb04217494d48d0ab093bf5b31db0921989#list-of-warnings ELECTRUM_LINTERS: E9,E101,E129,E273,E274,E703,E71,E722,F5,F6,F7,F8,W191,W29,B,B909 - ELECTRUM_LINTERS_IGNORE: B007,B009,B010,B019,B036,B042,F541,F841 + ELECTRUM_LINTERS_IGNORE: B007,B009,B010,B036,B042,F541,F841 - name: "linter: Flake8 Non-Mandatory" env: ELECTRUM_LINTERS: E,F,W,C90,B diff --git a/electrum/keystore.py b/electrum/keystore.py index b7f39c134..e1621855f 100644 --- a/electrum/keystore.py +++ b/electrum/keystore.py @@ -29,7 +29,7 @@ import hashlib import re import copy from typing import Tuple, TYPE_CHECKING, Union, Sequence, Optional, Dict, List, NamedTuple, Any, Type -from functools import lru_cache, wraps +from functools import wraps from abc import ABC, abstractmethod import electrum_ecc as ecc @@ -52,6 +52,7 @@ from .util import (InvalidPassword, WalletFileException, from .mnemonic import Mnemonic, Wordlist, calc_seed_type, is_seed from .plugin import run_hook from .logging import Logger +from .lrucache import LRUCache if TYPE_CHECKING: from .gui.common_qt.util import TaskThread @@ -398,6 +399,9 @@ class Deterministic_KeyStore(Software_KeyStore): class MasterPublicKeyMixin(ABC): + def __init__(self): + self._pubkey_cache = LRUCache(maxsize=10**4) # type: LRUCache[Sequence[int], bytes] # path->pubkey + @abstractmethod def get_master_public_key(self) -> str: pass @@ -434,8 +438,14 @@ class MasterPublicKeyMixin(ABC): def get_key_origin_info(self) -> Optional[KeyOriginInfo]: return None - @abstractmethod def derive_pubkey(self, for_change: int, n: int) -> bytes: + key = (for_change, n) + if key not in self._pubkey_cache: + self._pubkey_cache[key] = self._derive_pubkey(*key) + return self._pubkey_cache[key] + + @abstractmethod + def _derive_pubkey(self, for_change: int, n: int) -> bytes: """Returns pubkey at given path. May raise CannotDerivePubkey. """ @@ -501,6 +511,7 @@ class MasterPublicKeyMixin(ABC): class Xpub(MasterPublicKeyMixin): def __init__(self, *, derivation_prefix: str = None, root_fingerprint: str = None): + MasterPublicKeyMixin.__init__(self) self.xpub = None self.xpub_receive = None self.xpub_change = None @@ -608,8 +619,7 @@ class Xpub(MasterPublicKeyMixin): self._derivation_prefix = derivation_prefix self.is_requesting_to_be_rewritten_to_wallet_file = True - @lru_cache(maxsize=None) - def derive_pubkey(self, for_change: int, n: int) -> bytes: + def _derive_pubkey(self, for_change: int, n: int) -> bytes: for_change = int(for_change) if for_change not in (0, 1): raise CannotDerivePubkey("forbidden path") @@ -624,7 +634,7 @@ class Xpub(MasterPublicKeyMixin): return self.get_pubkey_from_xpub(xpub, (n,)) @classmethod - def get_pubkey_from_xpub(self, xpub: str, sequence) -> bytes: + def get_pubkey_from_xpub(cls, xpub: str, sequence) -> bytes: node = BIP32Node.from_xkey(xpub).subkey_at_public_derivation(sequence) return node.eckey.get_public_key_bytes(compressed=True) @@ -729,6 +739,7 @@ class Old_KeyStore(MasterPublicKeyMixin, Deterministic_KeyStore): type = 'old' def __init__(self, d: dict): + MasterPublicKeyMixin.__init__(self) Deterministic_KeyStore.__init__(self, d) self.mpk = d.get('mpk') # type: Optional[str] self._root_fingerprint = None @@ -807,8 +818,7 @@ class Old_KeyStore(MasterPublicKeyMixin, Deterministic_KeyStore): public_key = master_public_key + z*ecc.GENERATOR return public_key.get_public_key_bytes(compressed=False) - @lru_cache(maxsize=None) - def derive_pubkey(self, for_change, n) -> bytes: + def _derive_pubkey(self, for_change, n) -> bytes: for_change = int(for_change) if for_change not in (0, 1): raise CannotDerivePubkey("forbidden path")