Skip to content

fix(socket): forward CLibSocket::OnLost(int) to CEMSocket::OnClose so eD2k server/peer disconnects actually fire - #906

Merged
mrjimenez merged 1 commit into
amule-project:masterfrom
got3nks:fix/emsocket-onlost-dispatch
Jun 7, 2026
Merged

fix(socket): forward CLibSocket::OnLost(int) to CEMSocket::OnClose so eD2k server/peer disconnects actually fire#906
mrjimenez merged 1 commit into
amule-project:masterfrom
got3nks:fix/emsocket-onlost-dispatch

Conversation

@got3nks

@got3nks got3nks commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #905. Likely also resolves #393 (the long-standing "search hangs after server goes away") as the upstream cause.

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 (LibSocketAsio.cpp:638-655). The CLibSocket::OnLost(int) default is an empty no-op. The eD2k path classes — CServerSocketCEMSocketCEncryptedStreamSocketCSocketClientProxyCProxySocketCLibSocket — 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 added CECMuleSocket::OnLost(int) forwarding to the EC-side CECSocket::OnLost(). Same shape, different leaf chain.

Symptoms before the fix

  • eD2k server quits / drops connection → CServerSocket stays at CS_CONNECTED forever; next search hangs (mifritscher2's amule does not handle server connection aborts. #905 reproducer).
  • Peer drops the file-transfer TCP connection → CClientTCPSocket stays ES_CONNECTED; the upload slot stays held by a dead CUpDownClient; transfers show 0 KB/s until some unrelated timeout reaps it.

The fix

Override OnLost(int) on CEMSocket to call OnClose(int). OnClose is virtual, so dispatch reaches the per-class teardown:

  • CServerSocket::OnCloseSetConnectionState(CS_DISCONNECTED)serverconnect->ConnectionFailed() → cleanup + auto-reconnect.
  • CClientTCPSocket::OnCloseDisconnect() → 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 override keyword to seven adjacent CEMSocket members (OnError, OnSend, OnReceive, OnConnect, GetLastCalledSend, SendControlData, SendFileAndControlData, GetNeededBytes) that were missing it, to silence the -Winconsistent-missing-override warnings my new override would otherwise have spread across the file. All seven are legitimate overrides (verified against CLibSocket, CEncryptedStreamSocket, ThrottledFileSocket).

Test plan

  • macOS local build (Apple Silicon, Homebrew) — clean.
  • @mifritscher2 — would you mind retrying your original amule does not handle server connection aborts. #905 reproducer (amule + JEmule, search → quit server → re-search) against this branch? Build instructions are in the issue comment. With the fix you should see the "Connection lost" log line within a tick of the FIN, the connection state flip to disconnected, and the search request not get queued onto a half-closed socket. Auto-reconnect (if enabled) should kick in immediately.

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.
@mifritscher2

Copy link
Copy Markdown

I can confirm that this scenario works now, thanks :-)

There is another scenario, I'll write it in the issue.

@mrjimenez
mrjimenez merged commit eaba3f3 into amule-project:master Jun 7, 2026
7 checks passed
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.
@got3nks
got3nks deleted the fix/emsocket-onlost-dispatch branch June 8, 2026 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

amule does not handle server connection aborts. No search results after few minutes connected to server

3 participants