ClientTCPSocket: bypass download throttler for inbound server probes - #787
Merged
mrjimenez merged 1 commit intoMay 31, 2026
Merged
Conversation
…mule-project#778) When the ed2k server completes its HighID-callback connection back to us, it lands in CListenSocket::OnAccept and gets a CClientTCPSocket just like any peer. Under sustained download load that has the global throttler bucket exhausted, that socket's first read is deferred (pendingOnReceive = true) until budget refills, queued behind however many peer-side sockets are also waiting -- exactly the load profile mifritscher2 reported: the LowID/timeout pattern fires reliably at his configured ~350 kB/s cap, and not at fresh start or under light load. Server-side, the HighID verification timer is much tighter than the peer-throttler refill cadence. By the time amule actually processes the bytes on the probe connection, the server has already given up and assigned new_id == 0. That's then handled by the new_id == 0 branch of the OP_IDCHANGE handler (the silent `break` that exits without ever calling SetConnectionState; will be cleaned up in a follow-up so the failure mode is at least visible) and the connection times out 15 s later. Permanent LowID under sustained load is the net effect. The shape that fixes it is the same one we already use for the outbound CServerSocket since 356a59c / amule-project#393: skip the global download throttler for control traffic that's tiny and latency-sensitive. Extend the existing virtual: - CServerConnect::IsServerIP(uint32) -- true if the IP belongs to the currently-connected server or to any in-flight CServerSocket whose login attempt is still outstanding. Bounded by max_simcons (typically 2-3) so the loop cost is negligible. - CClientTCPSocket::IsDownloadThrottled() override -- returns false when m_remoteip matches IsServerIP. CClientTCPSocket sets m_remoteip in InitNetworkData immediately after the OnAccept upcall, so the override correctly identifies the server probe before its first OnReceive enters the throttler check. Peer-side throttling for actual file-transfer sockets is unchanged since IsServerIP returns false for any non-server-IP. The override fires per-call with one std::list walk in the worst case, which under typical max_simcons stays negligible.
This was referenced May 31, 2026
|
Seems to work, thanks :-) |
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.
got3nks
pushed a commit
to got3nks/amule
that referenced
this pull request
Aug 4, 2026
…-project#787) Splits the column-name/width/sort-order load-save logic out of CMuleListCtrl into a new widget-agnostic CListColumnStore, talking to the list widget through a small IColumnWidthProvider interface (column count, get/set width by index). Why: prep for the wxDataViewCtrl port of CSearchListCtrl (amule-project#675/amule-project#180 phase 2). got3nks asked that this extraction land as its own, independently reviewable commit, since the persistence logic (including the legacy pre-2.2.2 config migration path) must not be duplicated against wxDataViewColumn -- the new search list control will reuse CListColumnStore unchanged instead of reimplementing it. No new functionality: config keys ("/eMule/TableOrdering<name>", "/eMule/TableWidths<name>") and on-disk format are unchanged, so existing user settings keep working. CMuleListCtrl keeps its public API (SaveSettings/LoadSettings/ InsertColumn/SetTableName/GetColumnIndex/...) and now implements IColumnWidthProvider via explicit forwarding overrides -- needed because C++ won't otherwise link wxGenericListCtrl's GetColumnCount/GetColumnWidth/SetColumnWidth to the identically-named pure virtuals of an unrelated second base class. Verified: builds clean (macOS, full amule target); launched the built app against an isolated ~/aMuleTest config dir (own ports, never the real app) and confirmed column settings load and save back correctly, matching pre-refactor behaviour. Ran the local clang-tidy CI replica (Tier-1 whole-tree + Tier-2 changed-lines); Tier-1 reported no hits in the new/changed files, Tier-2 flagged narrowing conversions and a few modernize-* suggestions on the new code, all fixed.
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
3 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.
Summary
Fixes the LowID/timeout race surfaced on #778 (mifritscher2's diagnosis after the #779 assert fix). When the ed2k server completes its HighID-callback connection back to us, it lands in
CListenSocket::OnAcceptand gets aCClientTCPSocketlike any peer. Under sustained download load (mifritscher2's reproducible case was his ~350 kB/s cap), the global throttler bucket is exhausted and the first read on that socket is deferred (pendingOnReceive = true) until budget refills — queued behind however many peer-side sockets are also waiting.The server's HighID verification timer is much tighter than that refill cadence, so by the time amule processes bytes on the probe, the server has already moved on and assigned
new_id == 0. That's then routed through the silent-breakpath in theOP_IDCHANGEhandler (ServerSocket.cpp:300-311 — a separate UX bug to clean up in a follow-up), the connection times out 15 s later, and the net effect is permanent LowID under load.Confirmed by reporter: works at cold start / light load, fails reliably at the configured speed cap. Both match this load-correlated race exactly.
Fix
The shape is the one already used for the outbound
CServerSocketsince356a59c96/ #393: skip the global download throttler for control traffic that's tiny and latency-sensitive. Extend the existingIsDownloadThrottled()virtual:CServerConnect::IsServerIP(uint32)— true if the IP belongs to the currently-connected server or any in-flightCServerSocketwhose login attempt is still outstanding. Bounded bymax_simcons(typically 2-3), so the loop is negligible.CClientTCPSocket::IsDownloadThrottled()override — returns false whenm_remoteipmatchesIsServerIP.m_remoteipis set inInitNetworkData()immediately afterOnAccept, so the override correctly identifies the server probe before its firstOnReceiveenters the throttler check.Peer-side throttling for actual file-transfer sockets is unchanged.
Test plan
Follow-up
The silent-
breakinOP_IDCHANGE'snew_id == 0branch should be replaced with an explicit "server rejected ID assignment, disconnecting" path so even when the probe genuinely fails, the failure mode is visible instead of producing a silent 15 s wait. Separate PR once this lands.