http: rewrite CHTTPDownloadThread on top of wxWebRequest - #462

Summary

Fixes upstream PR #455's intermittent startup SIGSEGV on Linux by removing the entire wxHTTP/wxSocket-based HTTP download path and replacing it with an event-driven wxWebRequest-based implementation on the main thread. No worker thread, no wxEpollDispatcher on the download path, no recursive redirect handler, HTTPS transparent, no nested event loops — therefore no reentrancy surface.

Stock origin/master reproduces PR #455's crash at ~1-in-27 iterations of amuled in a tight startup loop. This branch cleared 115+ iterations clean on Linux/ARM64 without a single SIGSEGV.

Platform-specific follow-ups included in the same PR because they surfaced the moment the rewrite ran on each platform:

• macOS: Info.plist opt-out of App Transport Security, so plaintext http:// downloads still work (wxWebRequest's macOS backend is NSURLSession, which enforces ATS).
• macOS: std::_Exit(0) at the end of CamuleApp::OnExit to side-step a wx 3.3.2 upstream bug in ~wxWebSessionURLSession that otherwise crashes the process at teardown.
• Windows: CMake install ships an MSYS2 CA bundle next to .exe; CamuleApp::OnInit points CURL_CA_BUNDLE at it on startup. Without this, libcurl on MSYS2 builds fails every HTTPS request with error 77 on end-user machines.
• wxGTK: progress bar safety — clamp wxWebRequest::GetBytesExpectedToReceive() at event source and in the dialog, so a -1 (unknown Content-Length) never trips wxGauge::SetRange(-1) and the ensuing GTK assertion.

wx 3.2.0 is the floor (merged via #459), so wxWebRequest is always available in wxBase_net. No new build dependency.

Why the upstream crash happens

CHTTPDownloadThread::GetInputStream handles 301/302 redirects by recursively reopening with a fresh wxHTTP, tearing down the previous one with Destroy(). Since SourceForge and several other defaults now 301 to https://, and wxHTTP does not speak HTTPS, the recursion throws. Meanwhile the socket fd of the previous wxHTTP is still registered with wxEpollDispatcher, its wxSocketImpl still on wxPendingDelete. An epoll event fires on the fd between Destroy() and the pending-delete run → handler invoked on a socket mid-teardown → segfault.

libwx_baseu_net.so [handler on dead socket]
wxEpollDispatcher::Dispatch(int)
wxConsoleEventLoop::DispatchTimeout(unsigned long)

The fault is inside libwx_baseu_net — not fixable from aMule code paths alone. Avoiding the code path is the only reliable remedy.

Why wxWebRequest rather than libcurl-direct (PR #455's approach)

PR #455's libcurl backend is a good fix. Three reasons we reached for wxWebRequest instead:

• No new build dependency. wxWebRequest ships inside wxBase_net since wx 3.2.0 (distro minimum in cmake: bump MIN_WX_VERSION to 3.2.0 and retire legacy wx.cmake #459). Adds zero lines of CMake detection.
• Platform-appropriate backends for free. libcurl on Linux, WinHTTP on Windows, NSURLSession on macOS. Each uses the OS-native HTTP stack rather than a common userland library.
• Same underlying backend on Linux. wx's wxWebRequestCURL wraps libcurl, so Linux users get exactly the same HTTP engine PR http: fix intermittent startup SIGSEGV (and broken HTTPS) by using libcurl #455 reached for — just through a portable wx wrapper.

Why not a synchronous wrapper (nested wxEventLoop)

wxWebRequest in wx 3.2 is async-only; wxWebRequestSync lands in 3.3. A "sync wrapper" done by running a nested wxEventLoop while waiting for wxEVT_WEBREQUEST_STATE would reintroduce a classic reentrancy surface (modal dialogs running during the wait, menu dispatch during the wait, paint events mid-wait). We explicitly avoid this.

Instead, the class is event-driven on the main thread, and the 6 call sites of CHTTPDownloadThread already used an async-completion contract via wxEVT_CORE_FINISHED_HTTP_DOWNLOAD + CamuleApp::OnFinishedHTTPDownload. We preserve that dispatcher verbatim, so nothing changes for callers.

Once wx 3.3 / wxWebRequestSync is a reasonable distro floor, the entire class can collapse into a synchronous call on a worker thread with no wxSocket anywhere, and the current event-driven plumbing becomes a historical footnote. Create() / Run() / Stop() / OnExit() / Entry() are kept as stubs so the 6 call sites already pre-declare the eventual shape.

Reentrancy proof

• No nested event loops anywhere on the fix path. No ProcessEvents, no wxYield.
• No ShowModal — the progress dialog is Show(true) (modeless) as it always has been.
• No blocking wait on the request. Start() returns immediately; state transitions arrive asynchronously.
• Cancellation (Cancel button, StopAll) calls request.Cancel() fire-and-forget → returns → State_Cancelled event arrives later on the main loop.
• OnFinishedHTTPDownload dispatcher unchanged — ServerList::AutoDownloadFinished → AutoUpdate retry chain across addresses.dat URLs continues to work one-iteration-per-main-loop-turn.
• this->Destroy() (via CallAfter) runs only after State_Completed / Failed / Cancelled, so wxWebRequest's internal socket is already torn down by the time our handler is deleted.

What changed

Net: ~50 LOC added, ~260 LOC removed in HTTPDownload.cpp, one 8-line Info.plist injection, ~15 lines macOS exit guard + Windows CA-bundle hook, ~20 lines CMake install for the CA bundle. No CMake build-flag changes beyond the post-build plist injection. No new dep.

Proxy

HTTP proxy support via wxWebProxy::FromURL(...) on wx 3.3+, guarded by wxCHECK_VERSION(3,3,0) since wxWebProxy / SetProxy do not exist in 3.2. On wx 3.2 the libcurl backend still honours the standard http_proxy / https_proxy / all_proxy env vars automatically.

Not a regression vs. master: verified by reading wx 3.2.4 source — wxHTTP::SetProxyMode(bool) only flips a boolean and has no URL to route through unless wxURL::SetProxy / SetDefaultProxy is called first, which aMule never calls (git grep of entire upstream tree returns zero hits). The legacy Proxy-pref → HTTP plumbing was non-functional. SOCKS is out of scope on both paths; wx 3.3's wxWebProxy is HTTP-only and the legacy path didn't handle it either.

macOS: ATS opt-out

NSURLSession enforces App Transport Security, blocking plaintext http:// by default on 10.11+. The legacy wxSocket path sidestepped ATS because it was raw BSD sockets. First download after the rewrite reproduced:

HTTPDownload.cpp: HTTP download failed for
  http://upd.emule-security.org/server.met: The resource could not
  be loaded because the App Transport Security policy requires the
  use of a secure connection.

plutil -replace NSAppTransportSecurity -json '{"NSAllowsArbitraryLoads": true}' Info.plist as a POST_BUILD on the amule target restores the legacy behaviour. Not a new policy choice — simply keeping parity with master.

macOS: exit-time crash workaround

After the rewrite, Cmd+Q (or any shutdown after an HTTP download) aborts:

*** error for object 0x...: pointer being freed was not allocated
-[NSApplication terminate:] + 2004
exit + 44
__cxa_finalize_ranges
wxWebSession::~wxWebSession()
wxWebSessionURLSession::~wxWebSessionURLSession()

Root cause is an upstream wxWidgets 3.3.2 bug. Reading webrequest_urlsession.mm:508-515:

wxWebSessionURLSession::~wxWebSessionURLSession()
{
    [m_session release];
    [m_delegate release];
    ...
}

Per Apple's own docs, NSURLSession holds a strong reference to its delegate; -release on the session without first calling -invalidateAndCancel triggers the session's dealloc to drop the delegate ref — and the next line releases an already-freed delegate. Double-free. Fires in wx's module cleanup, in atexit's __cxa_finalize, and anywhere else the destructor runs.

Since aMule's orderly shutdown (CamuleApp::OnExit) has already saved prefs, credits, known files, shared files, server list, partfile state, joined upload/download/disk I/O/asio threads, and logged "aMule shutdown completed." before we reach the end of the function, nothing aMule-owned remains to clean up. std::_Exit(0) bypasses atexit + static destructors + wx's module cleanup, so the buggy dtor never fires and the process terminates cleanly.

Scope of the skip on macOS:

• AMULE_APP_BASE::OnExit() — default wxApp::OnExit, essentially a no-op.
• ~CamuleApp / ~CamuleAppCommon — only meaningful action is delete m_singleInstance, which drops the muleLock fcntl advisory lock. The kernel releases the lock on process exit anyway, so the next launch is unaffected.
• wx module cleanup — the point; WebRequestModule::OnExit is where the buggy dtor would fire.
• stdio buffer flush — logs have already been written line-by-line.
• atexit() handlers — grep atexit src/ returns zero hits; aMule registers none.

Linux and Windows continue through wx's normal cleanup; the _Exit block is #if defined(__APPLE__) only and can be removed the day wx ships a fix in a version we depend on. The upstream fix is straightforward (call -invalidateAndCancel before release).

Windows: portable install with bundled CA certs

On MSYS2 builds (MINGW64, CLANGARM64), wxWebRequest is backed by libcurl built against OpenSSL. MSYS2's libcurl is compiled with an absolute --with-ca-bundle= path — e.g. /clangarm64/etc/ssl/certs/ca-bundle.crt — that exists on the developer's machine but not on any end-user box. Result: every HTTPS request fails with

libcurl error 77: Problem with the SSL CA cert (path? access rights?)
Failed to download the version check file

This affects the default version-check URL (http://amule.sourceforge.net/lastversion → 301 HTTPS), and any server that redirects HTTP→HTTPS.

Two-part fix so cmake --install produces a self-contained portable directory that works on a fresh Windows machine:

1. CMake install-time bundling (src/CMakeLists.txt, in the existing WIN32 AND MINGW block that already uses file(GET_RUNTIME_DEPENDENCIES) to ship MSYS2 DLLs): find_file the MSYS2 CA bundle (/clangarm64/etc/ssl/cert.pem or /mingw64/etc/ssl/certs/ca-bundle.crt, as shipped by mingw-w64-*-ca-certificates), install(FILES ... RENAME ca-bundle.crt) into bin/ alongside the executables.

2. Runtime lookup (src/amule.cpp, __WINDOWS__-guarded, at the top of CamuleApp::OnInit): if CURL_CA_BUNDLE is not set, look for ca-bundle.crt next to the running executable via wxStandardPaths::GetExecutablePath + wxFileName::SetFullName, and set the env var via wxSetEnv. Users who want to point at their own bundle can still set the env var before launch.

Result: cmake --install build --prefix /path/to/portable produces a fully self-contained tree (GUI + daemon + CLI + 34 DLLs + skins + ca-bundle.crt) that runs on any Windows ARM64 or x86_64 machine without MSYS2, with HTTPS working out of the box.

Dialog / thread lifetime

CHTTPDownloadDialog holds a raw pointer to its owning CHTTPDownloadThread. On successful completion, FinishAndDestroy posts wxEVT_HTTP_SHUTDOWN to the dialog, posts wxEVT_CORE_FINISHED_HTTP_DOWNLOAD to the app, then CallAfters its own delete. Processing on the main loop:

1. Dialog's OnShutdown runs → Destroy() queues the dialog on wxPendingDelete.
2. CallAfter lambda runs → thread deleted.
3. Idle → DeletePendingObjects → ~CHTTPDownloadDialog → StopDownload() → dereferences m_owner — use-after-free.

Fix: OnShutdown nulls m_owner immediately, so the subsequent dtor sees m_owner == NULL and skips the whole StopDownload dance. The reverse direction (dialog closes first, cancels download) was already safe via DetachCompanion() on the thread side.

Progress bar safety (wxGTK)

wxWebRequest::GetBytesExpectedToReceive() returns wxInvalidOffset (= −1) when the server omits Content-Length, and can also return 0 early in the state machine before headers have been parsed. Forwarding either value unchanged to CHTTPDownloadDialog::UpdateGauge ended up in wxGauge::SetRange(-1), which puts m_rangeMax into an invalid state and trips an assertion on the next repaint on wxGTK:

./src/gtk/gauge.cpp(90): assert "0 <= m_gaugePos && m_gaugePos <= m_rangeMax"
  failed in DoSetGauge(): invalid gauge position in DoSetGauge()

Reproducible on the Ubuntu IP-filter update dialog. Fixed in two places:

• OnStateEvent clamps GetBytesExpectedToReceive() to 0 before posting the progress event, so "unknown total" never reaches the dialog as a negative value.
• UpdateGauge only touches the gauge when total > 0, and clamps current to [0, total] before SetValue, so out-of-range values can never reach the widget regardless of what the caller passes.

Follow-up / future work

• When wx 3.3 becomes a reasonable distro floor, the stub Create() / Run() / Stop() / OnExit() / Entry() can be turned into a real synchronous implementation via wxWebRequestSync, simplifying the class.
• Report the macOS ~wxWebSessionURLSession delegate double-release upstream to wx; drop the _Exit workaround when it lands.
• Optionally bridge aMule's Proxy prefs to http_proxy env vars on wx 3.2 via wxSetEnv, so GUI-configured proxy works on 3.2 builds too (out of scope here — legacy path never honoured the pref either).