EC client: defer modal popups out of OnPacketReceived call stack - #760
Merged
mrjimenez merged 1 commit intoMay 28, 2026
Merged
Conversation
CAddLinkHandler::HandlePacket and CCatHandler::HandlePacket popped
wxMessageBox synchronously inside the EC dispatch chain:
HandleRead -> CECSocket::OnInput -> ReadPacket -> OnPacketReceived
-> handler->HandlePacket -> wxMessageBox(modal)
The modal spins a nested wx event loop. That loop dispatches
CoreNotify_LibSocket* events, re-entering CECSocket::OnInput on
the same socket while the outer call still owns m_curr_rx_data /
m_bytes_needed / m_in_header. When the outer unwinds and resets
those, the rx state machine is left mid-frame; the next read
mis-parses a header and trips a protocol-error CloseSocket.
Failure is volume-sensitive: an idle client usually races through
the modal cleanly. A heavy shareset feeding INC_UPDATE traffic
between the EC_OP_FAILED reply and the modal dismissal makes the
race much more likely -- matching the Stoatwblr seedbox report in
amule-project#757 where a batched ADD_LINK with half-duplicates triggered
"External connection closed" on amuled right after the per-link
log lines.
Route both popups through wxTheApp->CallAfter so the dialog opens
after OnInput unwinds. CCatHandler's existing post-modal data
updates (path overwrite + downloadlistctrl refresh) now run before
the deferred dialog appears, which matches what the "keeping
directory '%s'" message already advertised.
Closed
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
May 28, 2026
… error
ReadHeader / ReadPacket call CloseSocket() on six different
protocol-error paths (oversize header, unauthorized resize, bad
packet flags, zlib init/free, ReadFromSocket failure). On amulegui
that route looks like:
asio HandleRead -> CECSocket::OnInput -> ReadPacket
-> CloseSocket() -> CAsioSocketImpl::Close()
CAsioSocketImpl::Close sets m_closed = true *before* the asio close,
so when HandleRead later fires with operation_aborted the
PostLostEvent gate at LibSocketAsio.cpp:695 is closed and OnLost
never bubbles up. The wrapper-OnLost dispatch fix landed in this
branch only covered the kernel-FIN leg; locally-initiated aborts
sit in the same wedge state amule-project#757 was about.
Stoatwblr's seedbox repro caught exactly this: the link-add
re-entrancy in amule-project#757/amule-project#760 corrupts the rx state machine, ReadPacket
logs "ReadPacket: error in packet read", we CloseSocket ourselves,
amuled sees the FIN and logs "External connection closed", and
amulegui sits silent on stale data because OnLost was suppressed.
Add CECSocket::CloseAndDispatchLost() (inline in ECSocket.h) that
follows InternalClose with a virtual OnLost call, and route all six
ReadHeader / ReadPacket protocol-error sites through it. The new
helper bypasses the m_closed gate by dispatching OnLost on the EC
layer directly -- the same shape DispatchSyncLost already uses for
sync clients. ProcessAuthPacket keeps using plain CloseSocket since
it already fires wxEVT_EC_CONNECTION itself.
The double-fire concern (asio's later operation_aborted hitting
PostLostEvent again) doesn't materialise: by the time the asio
HandleRead fires after our InternalClose, m_closed is true and
PostLostEvent skips -- the same gate that was suppressing the
useful first dispatch is exactly what suppresses the redundant
second one.
amuled side gets the same fix for free: CECServerSocket::OnLost
now runs on protocol-error closes too, so the m_ec_notifier
reference and per-client state are cleaned up immediately instead
of leaking until the connection times out.
6 tasks
mrjimenez
pushed a commit
that referenced
this pull request
May 28, 2026
… error
ReadHeader / ReadPacket call CloseSocket() on six different
protocol-error paths (oversize header, unauthorized resize, bad
packet flags, zlib init/free, ReadFromSocket failure). On amulegui
that route looks like:
asio HandleRead -> CECSocket::OnInput -> ReadPacket
-> CloseSocket() -> CAsioSocketImpl::Close()
CAsioSocketImpl::Close sets m_closed = true *before* the asio close,
so when HandleRead later fires with operation_aborted the
PostLostEvent gate at LibSocketAsio.cpp:695 is closed and OnLost
never bubbles up. The wrapper-OnLost dispatch fix landed in this
branch only covered the kernel-FIN leg; locally-initiated aborts
sit in the same wedge state #757 was about.
Stoatwblr's seedbox repro caught exactly this: the link-add
re-entrancy in #757/#760 corrupts the rx state machine, ReadPacket
logs "ReadPacket: error in packet read", we CloseSocket ourselves,
amuled sees the FIN and logs "External connection closed", and
amulegui sits silent on stale data because OnLost was suppressed.
Add CECSocket::CloseAndDispatchLost() (inline in ECSocket.h) that
follows InternalClose with a virtual OnLost call, and route all six
ReadHeader / ReadPacket protocol-error sites through it. The new
helper bypasses the m_closed gate by dispatching OnLost on the EC
layer directly -- the same shape DispatchSyncLost already uses for
sync clients. ProcessAuthPacket keeps using plain CloseSocket since
it already fires wxEVT_EC_CONNECTION itself.
The double-fire concern (asio's later operation_aborted hitting
PostLostEvent again) doesn't materialise: by the time the asio
HandleRead fires after our InternalClose, m_closed is true and
PostLostEvent skips -- the same gate that was suppressing the
useful first dispatch is exactly what suppresses the redundant
second one.
amuled side gets the same fix for free: CECServerSocket::OnLost
now runs on protocol-error closes too, so the m_ec_notifier
reference and per-client state are cleaned up immediately instead
of leaking until the connection times out.
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 4, 2026
…ect#760) Startup is slow and silent on a large share. The main window was created before the local work ran, but that work holds the main thread, so nothing painted until it finished: a rendered window that ignores every click, long enough that the OS offers to force-quit it. On a first run the wait extends well past the scan, into the hashing of everything the scan found unknown. Show a splash for the whole of it, and hold the main window back until it closes, so the window appears ready rather than appearing dead. The bar is driven by real totals rather than a spinner: part-file counts are exact, and the shared scan is sized against known.met, last session's view of the same tree, since counting first would walk it twice -- expensive exactly where it hurts, on network storage. With no known.met it is a first run, so the scan holds its band and the hashing phase spends it: that is where the time actually goes. Band weights come from measurements on a 10 000-file share (network 10 ms, 400 part files 130 ms, scan 6010 ms), and the phase timings stay in the log at normal level so a "startup is slow" report carries the numbers with it. Both list controls are batched for the duration. Each individually-sorted insert rebuilds the row index, so a burst of thousands is quadratic; this is the same BeginBatchUpdate the remote GUI uses for its startup EC reply. The splash is monolithic-only. amulegui's startup is an EC round trip with no local scan to wait on, and the daemon has no GUI: SplashScreen.cpp is kept out of its source list, and the app-side members and phases are compiled out. The backdrop is drawn rather than shipped -- a gradient plus the icon already embedded in the art provider -- so no new file enters the bundle.
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.
Summary
Companion to #758.
CAddLinkHandler::HandlePacketandCCatHandler::HandlePacketpoppedwxMessageBoxsynchronously inside the EC packet dispatch chain. The modal spins a nested wx event loop while it's up, which dispatchesCoreNotify_LibSocket*events that re-enterCECSocket::OnInputon the same socket. The outer OnInput still ownsm_curr_rx_data/m_bytes_needed/m_in_header; when it unwinds and resets those, the rx state machine is left mid-frame and the next read mis-parses a header into one of the protocol-errorCloseSocketpaths inReadHeader/ReadPacket.This is the most likely mechanism behind the link-add half of #757 (the "wedge" half is fixed in #758). The report: pasted batch of 12 links with 6 duplicates, amulegui pops the aggregated error popup, amuled logs the per-link
adding link '...'lines, then immediately logsExternal connection closed. Failure is volume-sensitive: with an idle peer the modal usually races through cleanly, but a heavy-shareset seedbox feeding INC_UPDATE notifications between the EC_OP_FAILED reply and the modal dismissal trips the race much more reliably.Fix
Route both modals through
wxTheApp->CallAfterso the dialog opens after OnInput unwinds. The packet contents are extracted into localwxStrings before the lambda capture, so the deferred callback doesn't reference the borrowed packet pointer.CCatHandlerpreviously didcs->path = ...andUpdateCategory/downloadlistctrl->Refresh()after its modal returned; those now run synchronously and the user sees the updates committed before the deferred dialog appears, which is consistent with what the "keeping directory '%s'" message already told the user.Test plan