fix(socket): forward CLibSocket::OnLost(int) to CEMSocket::OnClose so eD2k server/peer disconnects actually fire - #906
Merged
mrjimenez merged 1 commit intoJun 7, 2026
Conversation
The asio backend's HandleRead dispatches both clean close (FIN, bytes_transferred==0) and reset/read-error via PostLostEvent() -> socket->OnLost(0) through the CLibSocket vtable. The default CLibSocket::OnLost(int) is empty. The eD2k path classes (CServerSocket -> CEMSocket -> CEncryptedStreamSocket -> CSocketClientProxy -> CProxySocket -> CLibSocket) never overrode it, so peer FIN / RST was silently dropped: - CServerSocket stayed CS_CONNECTED after a server quit, hanging the next search (amule-project#905, amule-project#393). - CClientTCPSocket stayed ES_CONNECTED after a peer FIN, holding the upload slot with a dead CUpDownClient until some unrelated timeout reaped it ("transfer stuck at 0 KB/s"). eD2k twin of the EC bug fixed in e8462d6 / 4756556 / a6351ce (refs amule-project#757). Same fix shape: override OnLost(int) on the missing leaf base (CEMSocket) and forward into the per-class OnClose, which is virtual and already wired to the disconnect teardown that ran on the wxSocket close event before the asio migration. Also adds the override keyword to seven adjacent CEMSocket members that were missing it, to silence -Winconsistent-missing-override warnings that the new override would otherwise have spread across the file. All seven are legitimate overrides. Fixes amule-project#905.
|
I can confirm that this scenario works now, thanks :-) There is another scenario, I'll write it in the issue. |
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jun 7, 2026
…ule-project#912) Extends existing categories (preferring extensions over new lines): - Performance/Upload: amule-project#898 SlotAllocation default raised. - Networking & Discovery: wire-parser hardening list extended with amule-project#879/amule-project#882/amule-project#890/amule-project#886; new amuleweb security hardening bullet consolidating ngosang's amule-project#869-amule-project#874 triage (all landed in amule-project#875); amulegui list extended with amule-project#857; shared-folder watcher extended with amule-project#858. - Packaging: Windows installer i18n line extended with amule-project#899. - Internals & Refactoring: new docs-polish + code-quality bullets covering amule-project#851/amule-project#855/amule-project#862/amule-project#888/amule-project#900/amule-project#866/amule-project#867/amule-project#895 and amule-project#909/amule-project#910/amule-project#912. - Translations: new pre-release final-wave bullet covering amule-project#847/amule-project#856/ amule-project#891/amule-project#908/amule-project#860/amule-project#904/amule-project#859/amule-project#863/amule-project#861/amule-project#880/amule-project#911/amule-project#901/amule-project#902/amule-project#889/amule-project#868/amule-project#853. - Bug Fixes & Stability: amule-project#850/amule-project#854/amule-project#878/amule-project#906. - CI: ccache wiring (amule-project#892, amule-project#903) + CodeQL binutils-dev (amule-project#907). Contributors footer gains mifritscher and nguyenhoangminhhieu2004-gif (both first-time contributors). PR index extended through amule-project#912.
mrjimenez
pushed a commit
that referenced
this pull request
Jun 8, 2026
Extends existing categories (preferring extensions over new lines): - Performance/Upload: #898 SlotAllocation default raised. - Networking & Discovery: wire-parser hardening list extended with #879/#882/#890/#886; new amuleweb security hardening bullet consolidating ngosang's #869-#874 triage (all landed in #875); amulegui list extended with #857; shared-folder watcher extended with #858. - Packaging: Windows installer i18n line extended with #899. - Internals & Refactoring: new docs-polish + code-quality bullets covering #851/#855/#862/#888/#900/#866/#867/#895 and #909/#910/#912. - Translations: new pre-release final-wave bullet covering #847/#856/ #891/#908/#860/#904/#859/#863/#861/#880/#911/#901/#902/#889/#868/#853. - Bug Fixes & Stability: #850/#854/#878/#906. - CI: ccache wiring (#892, #903) + CodeQL binutils-dev (#907). Contributors footer gains mifritscher and nguyenhoangminhhieu2004-gif (both first-time contributors). PR index extended through #912.
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.
Fixes #905. Likely also resolves #393 (the long-standing "search hangs after server goes away") as the upstream cause.
The asio backend's
HandleReaddispatches both clean close (FIN,bytes_transferred==0) and reset/read-error viaPostLostEvent()→socket->OnLost(0)through theCLibSocketvtable (LibSocketAsio.cpp:638-655). TheCLibSocket::OnLost(int)default is an empty no-op. The eD2k path classes —CServerSocket→CEMSocket→CEncryptedStreamSocket→CSocketClientProxy→CProxySocket→CLibSocket— never overrode it, so peer FIN / RST was silently dropped on this side.This is the eD2k twin of the EC bug fixed in
e8462d619/4756556f2/a6351ce70(refs #757), which addedCECMuleSocket::OnLost(int)forwarding to the EC-sideCECSocket::OnLost(). Same shape, different leaf chain.Symptoms before the fix
CServerSocketstays atCS_CONNECTEDforever; next search hangs (mifritscher2's amule does not handle server connection aborts. #905 reproducer).CClientTCPSocketstaysES_CONNECTED; the upload slot stays held by a deadCUpDownClient; transfers show 0 KB/s until some unrelated timeout reaps it.The fix
Override
OnLost(int)onCEMSocketto callOnClose(int).OnCloseis virtual, so dispatch reaches the per-class teardown:CServerSocket::OnClose→SetConnectionState(CS_DISCONNECTED)→serverconnect->ConnectionFailed()→ cleanup + auto-reconnect.CClientTCPSocket::OnClose→Disconnect()→ releases the peer and frees the slot.Both teardown paths already existed for the (now-defunct) wxSocket close hook; this just wires the asio path into the same plumbing.
Also adds the
overridekeyword to seven adjacentCEMSocketmembers (OnError,OnSend,OnReceive,OnConnect,GetLastCalledSend,SendControlData,SendFileAndControlData,GetNeededBytes) that were missing it, to silence the-Winconsistent-missing-overridewarnings my newoverridewould otherwise have spread across the file. All seven are legitimate overrides (verified againstCLibSocket,CEncryptedStreamSocket,ThrottledFileSocket).Test plan