fix(ec): plug raw-pointer CECPacket leak on the notification send path - #797
Merged
Merged
Conversation
amule-project#765) Stoatwblr's amule-project#765 retention profile (jeprof --inuse_space --base on a ~3-hour amuled run) showed 329 MB / 41.8 % of all retained bytes sitting under CECServerSocket::OnPacketReceived → CECSocket::OnInput → CECTag::AddTag. The attribution looked like a request-path leak, but the real leak was on the notification send path -- reached recursively via SendPacket → WritePacket → OnOutput → WriteDoneAndQueueEmpty. jemalloc records the stack at allocation time, so the leaked notification CECPackets allocated way down the chain show up under OnInput's subtree. Two raw-pointer leaks, both with the same shape: CECPacket *packet = GetNextPacket(...); if (packet) { SendPacket(packet); } // packet leaks CECServerSocket::WriteDoneAndQueueEmpty (called from OnOutput at the end of every drain) and ECNotifier::NextPacketToSocket (called on every PartFile / SharedFile / Client / Search dirty-bit) both produce a fresh `new CECPacket(...)` and hand it to SendPacket. SendPacket takes a `const CECPacket*`, serialises the bytes into the per-socket output queue, and returns -- it does not own the packet, and the output queue stores CQueuedData (the serialised bytes), not the CECPacket itself. The two callers never delete. Compare with CECSocket::OnInput at libs/ec/cpp/ECSocket.cpp:412 which wraps the reply in CSmartPtr<const CECPacket> and gets it freed at scope exit -- exactly the pattern this commit applies to the two leaky sites. CSmartPtr is already a typedef for std::unique_ptr (libs/common/SmartPtr.h:56) and is pulled in transitively via the EC include chain. Volume per notification × frequency (every partfile dirty bit, every shared-file change, every search-result, every asio write completion, multiplied by connected GUI / web clients) makes this the dominant retained-bytes growth on a busy seeder. Matches the "RSS doubled when amulegui connected to amuled" / "most of it released on disconnect but some remained" observation in the early amule-project#765 reports.
Closed
2 tasks
1 task
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jun 4, 2026
…ndex Adds 55+ merged PRs to the 3.0.0 changelog since the last update (amule-project#747, 2026-05-27). Narrative additions cover: - Packaging: expanded the top list to include the macOS per-arch .app bundles and the Windows NSIS installer alongside the existing AppImage / Flatpak / .dmg / .zip entries. New bullets for amule-project#785 (alc/alcc/cas/wxcas everywhere + Windows amuleweb), amule-project#794 (.dmg amuleweb path), amule-project#789 (<OS>-<arch> artifact naming), amule-project#780 / amule-project#796 (Windows DPI + comctl32 manifest), amule-project#784 (FHS share/amule paths). - Bug Fixes & Stability: post-amule-project#744 fixes including EC notification leak (amule-project#797), big-library scaling (amule-project#736, amule-project#840 superseding amule-project#728), amulegui ghost entries (amule-project#810, amule-project#819, amule-project#841, amule-project#824, amule-project#830, amule-project#760), PartFile early hash (amule-project#762), server protocol fixes (amule-project#835, amule-project#788, amule-project#721, amule-project#787), crypto stream UB (amule-project#779), UAF prevention (amule-project#756), Kad rotation (amule-project#795, amule-project#799/amule-project#805), GTK warning silencing (amule-project#833, amule-project#826/amule-project#836), and the clang-tidy worklist (amule-project#770, amule-project#772-amule-project#774). - Translations: late-cycle wave covering French/Turkish manpages (amule-project#753/amule-project#754/amule-project#776), Galician (amule-project#763), Slovenian (amule-project#771), pt-BR (amule-project#768/amule-project#775/amule-project#812), French (amule-project#811), plus man-page tooling for date+version drift (amule-project#802). - Contributors: added ngosang for UX feedback on the late-3.0 cycle (amule-project#817/amule-project#818/amule-project#821/amule-project#828/amule-project#844) and ongoing work on the user-facing manual at amule-org.github.io. - Merged PRs flat index: extended with amule-project#746-amule-project#845 + amule-project#841.
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jun 4, 2026
…ndex Adds 55+ merged PRs to the 3.0.0 changelog since the last update (amule-project#747, 2026-05-27). Narrative additions cover: - Packaging: expanded the top list to include the macOS per-arch .app bundles and the Windows NSIS installer alongside the existing AppImage / Flatpak / .dmg / .zip entries. New bullets for amule-project#785 (alc/alcc/cas/wxcas everywhere + Windows amuleweb), amule-project#794 (.dmg amuleweb path), amule-project#789 (<OS>-<arch> artifact naming), amule-project#780 / amule-project#796 (Windows DPI + comctl32 manifest), amule-project#784 (FHS share/amule paths). - Bug Fixes & Stability: post-amule-project#744 fixes including EC notification leak (amule-project#797), big-library scaling (amule-project#736, amule-project#840 superseding amule-project#728), amulegui ghost entries (amule-project#810, amule-project#819, amule-project#841, amule-project#824, amule-project#830, amule-project#760), PartFile early hash (amule-project#762), server protocol fixes (amule-project#835, amule-project#788, amule-project#721, amule-project#787), crypto stream UB (amule-project#779), UAF prevention (amule-project#756), Kad rotation (amule-project#795, amule-project#799/amule-project#805), GTK warning silencing (amule-project#833, amule-project#826/amule-project#836), and the clang-tidy worklist (amule-project#770, amule-project#772-amule-project#774). - Translations: late-cycle wave covering French/Turkish manpages (amule-project#753/amule-project#754/amule-project#776), Galician (amule-project#763), Slovenian (amule-project#771), pt-BR (amule-project#768/amule-project#775/amule-project#812), French (amule-project#811), plus man-page tooling for date+version drift (amule-project#802). - Contributors: added ngosang for UX feedback on the late-3.0 cycle (amule-project#817/amule-project#818/amule-project#821/amule-project#828/amule-project#844) and ongoing work on the user-facing manual at amule-org.github.io. - Merged PRs flat index: extended with amule-project#746-amule-project#845 + amule-project#841.
mrjimenez
pushed a commit
that referenced
this pull request
Jun 4, 2026
Adds 55+ merged PRs to the 3.0.0 changelog since the last update (#747, 2026-05-27). Narrative additions cover: - Packaging: expanded the top list to include the macOS per-arch .app bundles and the Windows NSIS installer alongside the existing AppImage / Flatpak / .dmg / .zip entries. New bullets for #785 (alc/alcc/cas/wxcas everywhere + Windows amuleweb), #794 (.dmg amuleweb path), #789 (<OS>-<arch> artifact naming), #780 / #796 (Windows DPI + comctl32 manifest), #784 (FHS share/amule paths). - Bug Fixes & Stability: post-#744 fixes including EC notification leak (#797), big-library scaling (#736, #840 superseding #728), amulegui ghost entries (#810, #819, #841, #824, #830, #760), PartFile early hash (#762), server protocol fixes (#835, #788, #721, #787), crypto stream UB (#779), UAF prevention (#756), Kad rotation (#795, #799/#805), GTK warning silencing (#833, #826/#836), and the clang-tidy worklist (#770, #772-#774). - Translations: late-cycle wave covering French/Turkish manpages (#753/#754/#776), Galician (#763), Slovenian (#771), pt-BR (#768/#775/#812), French (#811), plus man-page tooling for date+version drift (#802). - Contributors: added ngosang for UX feedback on the late-3.0 cycle (#817/#818/#821/#828/#844) and ongoing work on the user-facing manual at amule-org.github.io. - Merged PRs flat index: extended with #746-#845 + #841.
mrjimenez
pushed a commit
to mrjimenez/amule
that referenced
this pull request
Aug 5, 2026
amule-project#797) The two encryption checks added in amule-project#785 end a login attempt by setting m_server_reply, closing the socket and returning false -- but they return before the tail of ProcessAuthPacket, which is the only place that fires wxEVT_EC_CONNECTION. Nothing else covers for them: plain CloseSocket deliberately does not dispatch OnLost, precisely because ProcessAuthPacket is documented as notifying for itself (see the note on CloseAndDispatchLost in ECSocket.h), and CAsioSocketImpl::Close suppresses the asio-side lost path as well. So amulegui never learned the attempt had ended. It waited out its connect-timeout watchdog and then reported "Unable to reach <host>:<port> ... check that aMule is running with External Connections enabled" -- wrong in every particular: the host is reachable, aMule is running, and EC is enabled. The accurate message, telling the user the core did not negotiate encryption and how to turn it off, sat unread in m_server_reply. This is the normal upgrade order -- amulegui updated before the daemon -- so it is what a user hits first. Route all three terminal paths through one NotifyConnectionResult helper. The mid-handshake EC_SALT_RECEIVED return still does not notify, which is correct: that attempt has not ended. Verified against a real pre-amule-project#785 core.
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
2 participants
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.
Fixes #765.
Root cause
Stoatwblr's jeprof
--inuse_space --baseprofile (full daemon run, ~3 hours, 786 MB net retention growth) pinned 329 MB / 41.8 % of retained bytes underThe call-site attribution is recursion:
OnInput → SendPacket(reply) → WritePacket → OnOutput → WriteDoneAndQueueEmpty → GetNextPacket → new CECPacket / new CECTag → leak. jemalloc records the stack at allocation time, so the leaked notification packets show up underOnInput's subtree even though the leakynewis deep down.Two sites, both same shape:
CECServerSocket::WriteDoneAndQueueEmptyatsrc/ExternalConn.cpp:366— called fromOnOutputat the end of every drain.ECNotifier::NextPacketToSocketatsrc/ExternalConn.cpp:2671— called per PartFile / SharedFile / Client / Search dirty-bit, multiplied by connected EC client count.CECSocket::SendPacket(const CECPacket*)atlibs/ec/cpp/ECSocket.cpp:301only serialises the packet's bytes into the per-socketm_output_queue(asCQueuedData, not asCECPacket) and returns. It does not own the packet. The input path handles ownership correctly viaCSmartPtr<const CECPacket>atECSocket.cpp:412— exactly the pattern applied here.Fix
Wrap both raw pointers in
CSmartPtr<CECPacket>(the codebase's typedef forstd::unique_ptr, already pulled in transitively via the EC include chain). Two two-line changes; no behaviour change, packets just get freed at scope exit instead of leaking.Volume
Per-notification × frequency:
CPartFile::SetDirty(download-progress refresh on every active download)CPartFile::AddFile/RemoveFileCSharedFilesadd/removeCClientstate transitionOnSendcompletion (which triggersWriteDoneAndQueueEmptyviaOnOutput)… multiplied by connected
amulegui/amulewebpeers. On Stoatwblr's seeder with persistent EC clients this added up to the dominant retained-bytes pattern by a wide margin.Reproducing the fix
The pre-fix amuled at his snapshot showed 329 MB of EC subtree retention over a 3-hour run. A post-fix
jeprof --base=<early>.heap --inuse_space --textover a comparable window should drop theCECServerSocket::OnPacketReceived/OnInputshares to single-digit %; what remains there will be the legitimate request/response working set.Refs #765.