kad: cap CKeyEntry::m_filenames so the list can't grow unbounded - #652
Merged
mrjimenez merged 1 commit intoMay 18, 2026
Merged
Conversation
…le-project#314) CKeyEntry::MergeIPsAndFilenames merges the incoming entry's filename list into the stored entry's, one sFileNameEntry per distinct publisher-chosen name. The publishing-IP list right next to it caps at 100 (Entry.cpp:429-432); the filename list never got the matching cap. A popular file accumulates one entry per distinct filename variant -- language renames, mirror prefixes, transliterations, case changes, trailing-paren copies -- and the list grows monotonically for the lifetime of the entry. ngosang's 20-min heaptrack of amuled (after 6 h warm-up, 7 TB shareset) showed ~2.36 MB leaked, with the leak headers overwhelmingly traced to: Kademlia::CEntry::sFileNameEntry(sFileNameEntry const&) std::list<sFileNameEntry>::push_front CEntry::SetFileName (Entry.cpp:118) CKademliaUDPListener::Process2PublishKeyRequest Extrapolated rate ~170 MB/day, matching the multi-day RSS creep the issue originally reported. Cap m_filenames at 100 entries (matching the m_publishingIPs cap below), popularity-ordered: when the list grows past the cap, sort by m_popularityIndex descending and resize. GetCommonFileName's "most popular wins" semantics are preserved since the survivors are exactly the entries with the highest popularity counts. The CIndexed keyword-to-file map is untouched. Search queries land there via keyword hash, not via per-file filename lists, and the KADEMLIAMAXINDEX = 60000 cap that protects it is unrelated. m_filenames inside each CKeyEntry only ever feeds GetCommonFileName for the displayed name in search-result rendering -- capping it doesn't change which files appear in results, only which name shows next to a file when multiple publishers used multiple names. eMule's reference code caps the equivalent list on the same shape; amule never ported that half when the 100-IP cap was added.
This was referenced May 25, 2026
ngosang
added a commit
to ngosang/amule
that referenced
this pull request
Jul 27, 2026
…mule-project#652) The Preferences view rendered every field emitted by the amuleapi /preferences endpoint except two, so the Web UI could neither show nor edit them even though both round-trip fine over EC: - files.create_normal (EC_TAG_FILES_CREATE_NORMAL, the inverse of /eMule/CreateSparseFiles). Left out by amule-project#599 as "INI-only", but it is a functional pref the daemon accepts on PATCH. Added to the Files tab next to alloc_full_size, its sibling disk-allocation policy. - core_tweaks.verbose (EC_TAG_CORETW_VERBOSE). It lives on the desktop Debugging page, which preferences.js deliberately skips; its real API category is core_tweaks, so it goes on the Advanced tab -- the same treatment files.mmap_enabled already gets there. Both are plain type: "bool" entries in TABS[], so buildField/collect handle them with no engine change, and the guest read-only path comes for free from the existing isGuest gate. en/es i18n keys added in lockstep. Auditing the Spanish side of amule-project#599 turned up two stale entries, fixed here with the wording the desktop catalog (po/es.po) already uses: - prefs_field_files_endgame was never translated and still read in English -> "Modo endgame: cambiar a fuentes mas rapidas para los bloques finales". - prefs_field_security_can_see_shares kept its old checkbox phrasing ("Permitir que otros vean mis archivos compartidos") after the field became a 3-state select, so the UI read "Permitir que otros vean mis archivos compartidos: [Todos]" -> "Quien puede ver mis archivos compartidos". check-i18n.mjs only compares keys and placeholders, so neither slipped past it; it stays green at 736 keys per locale and no prefs_field_* is left untranslated. Verified against a running daemon: GET returns both new fields, PATCH echoes and persists them, and re-parsing WritePreferencesBody() against TABS[] now reports no missing field in any of the 12 categories.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug investigation in #314.
@ngosang's 20-min heaptrack of amuled (after 6 h warm-up on a 7 TB shareset) showed leaks heavily concentrated in the Kademlia keyword-index code path:
Plus ~10 more entries in the top-30 leak list traced to the same handful of call sites in
KademliaUDPListener.cppandIndexed.cpp. Total leaked in 20 min: ~2.36 MB, which extrapolates to ~170 MB/day — consistent with the multi-day creep the issue originally reported.Root cause
CKeyEntry::MergeIPsAndFilenames(src/kademlia/kademlia/Entry.cpp:350) merges the incoming entry'sm_filenameslist into the stored entry's, onesFileNameEntryper distinct publisher-chosen name. The publishing-IP list right next to it caps at 100 (:429-432); the filename list never got the matching cap. A popular file accumulates one entry per distinct filename variant — language renames, mirror prefixes, transliterations, case changes, trailing-paren copies — and the list grows monotonically for the lifetime of the entry.GetCommonFileNamealready walks the list and returns the highest-m_popularityIndexentry as the displayed name, so the long-tail filename variants past a few dozen are read-only ballast.Fix
Cap
m_filenamesat 100 entries (matching them_publishingIPscap), popularity-ordered: when the list grows past the cap, sort bym_popularityIndexdescending and resize.GetCommonFileName's "most popular wins" semantics are preserved since the survivors are exactly the entries with the highest popularity counts.What this doesn't change
CIndexed(the actual keyword → file index) is untouched. Search queries land there via keyword hash, not via per-file filename lists, and theKADEMLIAMAXINDEX = 60000cap that protects it is unrelated. The filename list inside eachCKeyEntryonly ever feedsGetCommonFileName()for the displayed name in search-result rendering — capping it doesn't change which files appear or whether amuled answers a search, only which name shows next to a file when multiple publishers used multiple names.eMule's reference code caps the equivalent list on the same shape; we just never ported that half when the 100-IP cap below was added.