Skip to content

http: fix intermittent startup SIGSEGV (and broken HTTPS) by using libcurl - #455

Closed
got3nks wants to merge 4 commits into
amule-project:masterfrom
got3nks:http-libcurl-pr
Closed

http: fix intermittent startup SIGSEGV (and broken HTTPS) by using libcurl#455
got3nks wants to merge 4 commits into
amule-project:masterfrom
got3nks:http-libcurl-pr

Conversation

@got3nks

@got3nks got3nks commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes an intermittent SIGSEGV on Linux during amuled / amule startup. The crash happens inside wxEpollDispatcher::Dispatch when a startup HTTP download (version check, server.met, nodes.dat, ...) 301/302-redirects to an https:// URL — which wxHTTP can't speak, so it throws from inside its redirect handler and the racy socket teardown leaves the dispatcher pointing at a dead handler.

The fix adds an optional libcurl backend. When libcurl is found at configure time, the startup HTTP path no longer touches wxSocket/wxEpollDispatcher at all, so the crash (and the HTTPS limitation that causes it) both go away. Falls back to the existing wxHTTP code when libcurl is not available — no behaviour change for that build.


Why the crash happens

wxHTTP does not support HTTPS, and several of the default URLs baked into aMule now 301/302 redirect to https:// (SourceForge's HTTPS-everywhere policy). In CHTTPDownloadThread::GetInputStream:

  1. wxHTTP::Destroy() is called on the current handler → schedules deferred deletion via wxPendingDelete; the underlying wxSocketImpl fd stays registered with wxEpollDispatcher.
  2. GetInputStream is re-entered recursively with the new https:// URL and throws "Protocol not supported for HTTP download: https".
  3. Before the deferred delete runs, an epoll event fires on that still-registered fd. The dispatcher invokes a method on a socket mid-teardown → crash.

Stack signature (addresses vary):

libwx_baseu_net-3.2.so.0 [0xc6f35fa79a]          ← handler on dead socket
wxEpollDispatcher::Dispatch(int)
wxConsoleEventLoop::DispatchTimeout(unsigned long)
...

Sometimes preceded by:

Failed to unregister descriptor 12 from epoll descriptor 15 (error 2: No such file or directory)

Without libcurl this crash is reproducible on Ubuntu/GTK3 amuled from stock origin/master: ~1-in-27 runs in a tight startup loop on idle hardware, more frequent (1-in-~5) with other startup activity. Any downstream patch that increases startup concurrency makes it worse. Because the crash happens inside wxBase_net it cannot be fixed from aMule code paths alone — avoiding the code path is the only reliable remedy.

Why libcurl

libcurl has no interaction with wxEpollDispatcher or wxPendingDelete. A worker thread makes a blocking curl_easy_perform() call, gets data through C callbacks, and returns. No wx event system, no deferred deletion, no race. Bonus: full proxy control (SOCKS types and auth), which wxHTTP::SetProxyMode() didn't expose even though aMule's config has always stored them.


What

CMakeLists.txt

  • Optional find_package(CURL). Defines HAVE_LIBCURL when found; logs the active backend.

src/CMakeLists.txt

  • Links CURL::libcurl to amuled and amule when HAVE_LIBCURL is set.

config.h.cm

  • Adds #cmakedefine HAVE_LIBCURL 1.

src/HTTPDownload.h, src/HTTPDownload.cpp

Three #ifdef HAVE_LIBCURL static member functions on CHTTPDownloadThread:

  • CurlWriteCallback — streams response body to wxFFileOutputStream.
  • CurlProgressCallback — posts wxEVT_HTTP_PROGRESS for the GUI dialog, handles cancellation via TestDestroy().
  • DoDownloadCurl(int& response, int& error) — runs the request: follows redirects, sends If-Modified-Since, handles proxy (HTTP/SOCKS4/SOCKS4a/SOCKS5 + auth, all read from the existing CProxyData prefs), times out, reports status.

Entry() dispatches to DoDownloadCurl() when HAVE_LIBCURL is defined; otherwise the existing wxHTTP path runs unchanged.

Everything is scoped to the class — no public accessors, no free helpers.


Design note: why not wxWebRequest

Before settling on libcurl directly, I ported the download to wxWebRequest (wx 3.1.5+; backed by libcurl on Linux, WinHTTP on Windows, NSURLSession on macOS). On paper the ideal fix: platform-native stack, HTTPS support, no direct wxSocket involvement.

In practice it still crashed, 1-in-~6 runs, with a different stack but the same class of race:

libcurl.so.4 (various)
curl_multi_socket_action
libwx_baseu_net-3.2.so.0                  ← wxWebRequest backend
wxEvtHandler::ProcessEvent
wxEpollDispatcher::Dispatch(int)

wxWebRequest is async-only in wx 3.1.5/3.2 (sync wrapper only in wx 3.3+). Driving it from a worker thread required either a nested wxEventLoop (which races against the main thread's loop over the shared libcurl multi handle) or posting work back to the main thread (significant refactor). Events fire on whichever thread happens to be dispatching, and the lifetime of our local wxEvtHandler ended up as fragile as the wxHTTP path. Different code path, same class of race.

Using libcurl directly avoids all of this.


Backward compatibility

  • Linux with libcurl dev package (Debian/Ubuntu libcurl4-openssl-dev, Fedora libcurl-devel, etc.): automatic pickup, HTTPS works, no startup crash.
  • Systems without libcurl: build unchanged from master. wxHTTP path used; the startup crash remains possible (this PR does not regress that behaviour).
  • Distro maintainers who want the fix shipped should add libcurl-dev to aMule's build dependencies.
  • No change to MIN_WX_VERSION, command-line flags, or config keys.

Reproducing the crash

The following script launches amuled in a loop and stops when it segfaults (139) or aborts (134). On a stock origin/master Ubuntu/GTK3 build without libcurl it typically hits a crash within a few dozen iterations. With this PR (and libcurl present at configure time) it runs indefinitely.

#!/bin/bash
BIN=${1:-$HOME/amuled}
RUN=0
echo "Binary: $BIN"
while true; do
    RUN=$((RUN+1))
    echo "=== Run #$RUN at $(date +%H:%M:%S) ==="
    rm -f ~/.aMule/muleLock
    # No redirection — output goes straight to terminal so the crash
    # dump isn't lost to stdio buffering when the process aborts.
    timeout 6 "$BIN" -o
    RC=$?
    if [ $RC -eq 139 ] || [ $RC -eq 134 ]; then
        echo ">>> CRASHED on run #$RUN (rc=$RC)"
        exit 0
    fi
    if [ $RC -ne 124 ] && [ $RC -ne 0 ]; then
        echo ">>> unexpected rc=$RC, stopping"
        exit 1
    fi
    sleep 0.3
done

Usage: ./crash-hunt.sh /path/to/amuled (defaults to ~/amuled).

wxHTTP does not support HTTPS. The startup HTTP downloads (version check,
server.met, nodes.dat, ipfilter, GeoIP) hit SourceForge URLs that 301-redirect
to HTTPS, and the redirect-to-unsupported-protocol path in HTTPDownload.cpp
triggers a wxSocket lifecycle race in wxWidgets 3.2 — observed as an
intermittent SIGSEGV in libwx_baseu_net during wxEpollDispatcher dispatch at
aMule startup.

Add an optional libcurl-based implementation. When libcurl is found at
configure time, HTTPDownload.cpp uses it instead of wxHTTP: HTTPS works
natively, redirects are followed transparently, and the download runs
entirely within the worker thread with no wxSocket or wxEpollDispatcher
interaction, eliminating the race. Falls back cleanly to the existing
wxHTTP code when libcurl is not available.

- CMakeLists.txt: optional find_package(CURL)
- config.h.cm: HAVE_LIBCURL define
- src/CMakeLists.txt: link CURL::libcurl to amuled / amule when found
- src/HTTPDownload.cpp: libcurl worker (curl_easy_perform with redirect
  following, if-modified-since, progress callback for the GUI dialog, and
  cancellation via TestDestroy())
- src/HTTPDownload.h: expose GetCompanion() and FormatDateHTTP() to the
  libcurl free functions

No behaviour change on systems without libcurl.
@Vollstrecker

Copy link
Copy Markdown
Collaborator

Point 1: I'm no friend of maybe. If you want libcurl support, set an option to indicate that and then it is required or don't set the option and don't get support. Noone wants to ask if it was built with a feature and the user says: maybe.

Point 2: Why a new dep if wx provides a solution? Either we want wx, then we use it, or we want to migrate away from it, then we should start with that.

got3nks added 2 commits April 20, 2026 16:32
Replace auto-detection (find_package(CURL) — optional, defaults depending
on system) with a user-facing CMake option ENABLE_LIBCURL defaulting to
ON. Fails at configure time with a clear error if libcurl is not found
instead of silently falling back, and lets packagers opt out with
-DENABLE_LIBCURL=OFF.

Addresses review feedback: no "maybe, depending on system" state,
reproducible builds clear from the cache.
find_package(CURL REQUIRED) now fails at configure time with
ENABLE_LIBCURL=ON (the default), so the cmake job needs the
library available.
@got3nks

got3nks commented Apr 20, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Responding to both points:

Point 1 (auto-detection is "maybe") — Agreed. Pushed a follow-up commit (a5c46a96a) that replaces auto-detection with an explicit CMake option ENABLE_LIBCURL (default ON). When ON and libcurl is not found, the configure step errors out with a clear message; users can explicitly opt out with -DENABLE_LIBCURL=OFF. No more "maybe" state.

Point 2 (why a new dep if wx provides a solution?) — I agree in principle, but the choice of which wx API matters:

wxHTTP is what's currently used, and it doesn't speak HTTPS at all. The startup crash this PR fixes is specifically caused by wxHTTP's redirect-to-HTTPS path. So "stay with wx" in the current sense means staying with a broken HTTP client that can't reach the URLs aMule ships with (SourceForge).

wxWebRequest (wx ≥ 3.1.5) is the modern replacement and supports HTTPS. I tried it first — see the "Design note: why not wxWebRequest" section in the PR body. Two problems:

  • It's async-only in wx 3.1.5/3.2 (the sync wrapper only lands in wx 3.3). Driving it from our HTTP worker thread produced a different flavour of the same dispatcher race (stack trace in the PR body). Fixing it properly would require either porting the HTTP subsystem to run on the main thread, or requiring wx 3.3 (bumping MIN_WX_VERSION from 2.8.12 to 3.3.0 — much bigger impact than adding libcurl).
  • On Linux, wxWebRequest's backend is libcurl. Using wxWebRequest on Linux pulls in libcurl anyway; we'd just be going through an extra wx layer (the one that has the race). On macOS/Windows wxWebRequest uses native APIs, but on Linux — the platform where the crash reproduces — it's libcurl either way.

So on our crashing platform the real choice is: libcurl directly (this PR) vs. libcurl via a wx wrapper that reintroduces the race. Both require the libcurl dependency.

If you'd prefer to go the wxWebRequest route anyway, I'm happy to redo the PR on top of a MIN_WX_VERSION bump to 3.3 (where the sync API is available) — just want to flag that the dependency surface is similar and the migration scope is larger.

HTTPDownload.cpp is in COMMON_SOURCES, so it is compiled into all three
executables (amuled, amule, amulegui). With HAVE_LIBCURL defined globally
via config.h, its libcurl calls are present in every object file — so
amulegui also needs the link, otherwise it fails with:

    undefined reference to symbol 'curl_easy_cleanup@@CURL_OPENSSL_4'
    /lib/x86_64-linux-gnu/libcurl.so.4: error adding symbols:
    DSO missing from command line
@got3nks got3nks changed the title http: fix intermittent startup SIGSEGV (and broken HTTPS) by using libcurl when available http: fix intermittent startup SIGSEGV (and broken HTTPS) by using libcurl Apr 20, 2026
@Vollstrecker

Copy link
Copy Markdown
Collaborator

k, so if I get you right this is caused by old URLs hardcoded and we know it chrashed, so there's not really a choice of not using https.

And yes, I also don't like the idea of staying on a long-time dead version of wx, so I see no use in working around problems newer wx has already solved. If wx-3.3 can do all this, then we should change to there and if possible drop relying on wx providing compat-stuff for abandoned things.

@got3nks

got3nks commented Apr 20, 2026

Copy link
Copy Markdown
Contributor Author

Good to confirm we're aligned on needing HTTPS and not wanting to stay on dead wx.

Quick data point on "migrate to wx 3.3" timing though: wxWidgets 3.3.0 was released on 2025-06-06 (~10 months ago), and no mainstream Linux distro has packaged it yet:

Distro wx version
Ubuntu 24.04 LTS / 25.04 / 25.10 3.2
Debian 12 / 13 / sid 3.2

Bumping MIN_WX_VERSION to 3.3 right now would make aMule un-installable from the system package manager on every current Linux distro — users would have to build wx 3.3 from source first, or wait for the next distro cycles (probably Ubuntu 26.04 LTS).

@Vollstrecker

Copy link
Copy Markdown
Collaborator

As it's a year old, I guess someone there is aware of a new release and it will arrive soon. For the mean-time, I plan for the near future to update CmDaB to more uptodate stuff. It is able to support pupnp in it's build-chain and beside upnp I managed to bring crypto++ and zlib in a shape that's useable with it. I started with direct deps, but I'm pretty sure even when it doesn't meet all requirements I can integrate wx there aswell (was planned anyways, just not that fast).

I always say if anyone wants to build a software, building the needed deps is not asked too much if they are not present.

@mifritscher2

mifritscher2 commented Apr 20, 2026

Copy link
Copy Markdown

What about using libcurl directly on Linux for be time being (the only platform in which compiling occours by many in reality - and the Linux port of wx does use libcurl anyway) and using wxWebRequest for Windows and MacOS?

When wx 3.3 + is on most stable / LTS builds Linux could switch as well. Raising depend versions beyond the versions the current Linux distros offer do complicate e.g. backports (Debian has e.g. debian-backports) as well. Additionally, this PR could qualify even as bugfix which current distros could integrate - which is not possible if library version dependencies are rising...

@got3nks got3nks mentioned this pull request Apr 20, 2026
@Vollstrecker

Copy link
Copy Markdown
Collaborator

Sure, establishing double structures when using a toolkit that's abstracting away the platform is always the best idea, maybe we should mark it as temporary and discuss in 10 years if the purpose is still valid.

There are many issues to solve before a new release, plenty of time even for Debian to close that gap. All other platforms have wheelchairs like homebrew and vcpkg which shouldn't be used by anyone but ship wx-3.3.

When it comes to new releases, the reasoning shouldn't be what currently ships on Distros (especially Debian, and yes I'm still on Debian testing, so I know the pain). It should be about what is available and serves the purpose, noone benefits when now introducing a workaround instead of waiting some time.

Talking about backports: there's no guarantee it will be part of that just because it's possible, most stuff there is useful for many people and therefore someone took the time to bring it there, A bugfix in ten years won't motivate anyone to do that workm especially as it's not security relevant.

@got3nks

got3nks commented Apr 20, 2026

Copy link
Copy Markdown
Contributor Author

@Vollstrecker Thanks for the CmDaB context, nicely self-contained for source builders.

Small pushback though: CmDaB solves the dep story for people building from source, but it doesn't help distro packagers: their build farms forbid fetching sources at build time, so Debian/Ubuntu/Arch/Fedora packages must link against system-available libs.
If MIN_WX_VERSION=3.3 lands before distros ship wx 3.3, the concrete outcome isn't "users build wx themselves", it's that distros keep shipping the already-packaged 2.3.3. So apt install amule users stay frozen on 5-year-old code for another 1–2 years, missing everything that's accumulated, until Ubuntu 26.04 LTS / Debian 14 catch up.

Not arguing "don't ever move to wx 3.3", just that CmDaB doesn't close that distro gap and the cost of bumping MIN_WX_VERSION is paid by exactly the users who don't build from source.

Broader question: last release was 2.3.3, Feb 2021. What do you consider the 2–3 most important things to land before the next release? I'd rather direct effort where it actually moves a release forward.

#455 can sit open, be closed, or merged as an optional fallback, your call.

@Vollstrecker

Copy link
Copy Markdown
Collaborator

Importance-order:

  • wx-3 support without relying on compat-mode (they can't keep this running forever)
  • Crashes and mem-leaks (many are hopefully solved by step 1)

Wish-list: if wx-sockets has also improved over time: Deactivation of boost.

For the https fix, I guess this can sit open for now. I don't have the time to check now, but maybe it's not a that big change so we could pull that one from wx-3.3 and place it under extern/wxwidgets/, so it can be used or ignored automatically when releases hit distros.

@Vollstrecker

Copy link
Copy Markdown
Collaborator

If I get you right, the objection against wxWebRequest is mainly that it is asynchronous.

Given the analysis above, the actual issue isn’t async execution, but the wxHTTP code path interacting badly with deferred destruction and epoll once HTTPS redirects are involved. Avoiding that path altogether is the key.

From my perspective, wrapping wxWebRequest into a synchronous wait loop would already be an improvement over the current situation: we keep the blocking semantics we already depend on, but move them onto an API that is meant to deal with modern HTTP/HTTPS behaviour.

Longer‑term, I’d strongly prefer adapting the handlers to the event‑driven network model wxWidgets is clearly moving towards. We didn’t choose wx only to drop down to native networking abstractions again, and if aMule is going to be actively maintained going forward, I’d like to avoid relying on compatibility layers whose behaviour is becoming increasingly fragile. The current wxHTTP path works today, but it’s unlikely to remain sustainable.

@mrjimenez

Copy link
Copy Markdown
Contributor

Is it worthy to keep this open when it is a clear fix to a problem we already have?
I don't think it is reasonable to wait for a new wx to be in distros, that makes this software hard to be used.

@Vollstrecker

Copy link
Copy Markdown
Collaborator

For webRequests wrapped to be sync the current wx has everything needed. And I guess that's better than a new dep.

@got3nks

got3nks commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

I'm actively working on raising MIN_WX_VERSION from 2.8.12 to 3.2 (the version shipped by every current Linux distro) as the first step. Both HTTP options below assume that bump has landed, since wxWebRequest itself only exists from wx 3.1.5.

I went through the aMule HTTP surface to be concrete:

HTTP call-site inventory (all go through CHTTPDownloadThreadOnFinishedHTTPDownload):

# Site File URL Context
1 IP filter update IPFilter.cpp:483 user-configured dialog, user-triggered
2 Server list (manual) ServerList.cpp:838 user-configured background, user-triggered
3 Server list (auto) ServerList.cpp:888 list of URLs with retry chain startup if AutoServerlist()
4 Version check amule.cpp:564 http://amule.sourceforge.net/lastversion startup
5 GeoIP DB IP2Country.cpp:99 https://mailfud.org/geoip-legacy/GeoIP.dat.gz startup / dialog, only HTTPS URL
6 nodes.dat amule.cpp:2053 user-configured dialog, user-triggered (Kad)

All six currently run on a detached/joinable wxThread, completion delivered via a single wxPostEvent(wxTheApp, wxEVT_CORE_FINISHED_HTTP_DOWNLOAD) → one dispatcher. So the surface is manageable: 1 class + 6 consumers.

On "wrap wxWebRequest to be sync — current wx has everything needed": the scope is larger than it sounds, because of the wx 3.2 vs 3.3 API split.

  • wx 3.2 (what distros ship): wxWebRequest is async/event-driven only. No Execute(), no blocking Start(). "Sync wrapper on current wx" means running a nested wxEventLoop on the main thread while waiting for wxEVT_WEBREQUEST_STATE — which reintroduces reentrancy (modal dialogs, menu events, repaint-during-wait) and is itself a known category of bug. The clean path on 3.2 is to refactor the 6 call sites to be event-driven on the main thread: rewrite CHTTPDownloadThread to own a wxWebRequest, make progress dialogs (3 of 6 sites) modeless listeners of state events, preserve the existing OnFinishedHTTPDownload dispatcher. Non-trivial, touches the 3 progress dialogs and the retry chain in ServerList::AutoDownload.
  • wx 3.3 added wxWebRequestSync with a blocking Execute() specifically for worker-thread use. On 3.3 the migration is almost mechanical: swap wxHTTP for wxWebRequestSync inside CHTTPDownloadThread and we're done — ~1 class changed, 6 consumers untouched.

So the three realistic options:

  1. Keep PR http: fix intermittent startup SIGSEGV (and broken HTTPS) by using libcurl #455 (libcurl) — minimal change, unblocks the startup-crash bug today on every wx version we support (including the current 2.8.12 floor).
  2. Bump MIN_WX_VERSION to 3.2 and refactor HTTP to event-driven on the main thread — ~1 class + 3 dialogs rewritten, solves crash without a new dep, still usable on current distros. Requires the wx-3.2 bump first (we're already working on it).
  3. Bump MIN_WX_VERSION to 3.3 and use wxWebRequestSync — cleanest code, but no mainstream distro currently ships wx 3.3 (Debian 13 = 3.2, Ubuntu 25.10 = 3.2), so apt install amule would go backwards for 1–2 years until distros catch up. I'd strongly disagree with 3.3 as a hard minimum right now for that reason.

My read: do (1) as a targeted fix for the immediate crash, plan (2) as part of the post-3.2-bump cleanup when we're actively reshaping network code anyway, and revisit (3) when distros ship 3.3.

@Vollstrecker

Copy link
Copy Markdown
Collaborator

As option 2 works, option 3 is out, and as you actively work on the wx-3.2 transition option 1 would mean a quick fix with a new dep that is needed only until the migration is done. We all know it won't be needed then and we all know noone will remove when it's obsolete.

I clearly vote for option 2 (maybe with preparation to ease transition to option 3 when it's releases). The last bugfix is 5 years old, the last real release much older. I see no benefit in doing this quick instead of doing it right just to be some weeks faster. Transitioning to wx3 would justify a new major version, that should be the goal.

And yes, I know this stuff is a real pain for users, but it seems here's some momentum and if just the pain is fixed I'm afraid it will stall again.

@got3nks

got3nks commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

I clearly vote for option 2 (maybe with preparation to ease transition to option 3 when it's releases). The last bugfix is 5 years old, the last real release much older. I see no benefit in doing this quick instead of doing it right just to be some weeks faster. Transitioning to wx3 would justify a new major version, that should be the goal.

I'll submit the MIN_WX_VERSION bump to 3.2 PR as soon as it's ready, then follow with the HTTP refactor using wxWebRequest (event-driven on the main thread, per option 2) which can supersede this PR.

@mrjimenez

Copy link
Copy Markdown
Contributor

Just my next two cents.

  • I don't know the reasons why wx 3.3 has not been put inside new distros, but I see no short term sign that this is changing, so we might need to coexist with that situation for some time.
  • We seem to have a quick solution. Ok, introduces a new dep. But is also optional, if you believe you can coexist with a crash.
  • Removing the libcurl dependency should be very easy, since it is all inside #ifdef's HAS_LIBCURL . Just add this task to the TODO list when wx 3.3 is out.
  • Option 2 seems to be a lot of work that will no longer make sense in wx 3.3. In my opinion, I might be wrong on this one, but removing this work later seems to be more difficult than removing the libcurl dependency.

@Vollstrecker

Copy link
Copy Markdown
Collaborator

Just my next two cents.

* I don't know the reasons why wx 3.3 has not been put inside new distros, but I see no short term sign that this is changing, so we might need to coexist with that situation for some time.

For me it seems like 3.3 is the same as 2.9 in the old days - Dev version that will get 3.4 when stable. That's why it's off the table.

* We seem to have a quick solution. Ok, introduces a new dep. But is also optional, if you believe you can coexist with a crash.

As 3.2 has everything to solve the crash and is available, I just see no reason to do it not with wx.

* Removing the libcurl dependency should be very easy, since it is all inside `#ifdef's HAS_LIBCURL` . Just add this task to the TODO list when wx 3.3 is out.

Sure, and this works like that. You know that a) contributors come and go, and although it seems that got3nks isn't to drop code and disappear, we're talking about more than some weeks and life's a bitch and b) yes it is guarded by ifdef's, but it's curl or crash, so this will get to a better don't touch it as long as it works.

* Option 2 seems to be a lot of work that will no longer make sense in wx 3.3. In my opinion, I might be wrong on this one, but removing this work later seems to be more difficult than removing the libcurl dependency.

More work, sure. But it's part of a bigger transition to native wx-3.2 support. The stuff in wx-3.3 seems to add functionality, the way this is implemented in 3.2 works also there (yes the 3.3 is easier then, but doesn't superseed), so 3.2 as base should work with 3.3 (when it's 3.4) and I'm pretty sure it will either continue to do so throughout the 3.x lifecycle or when there will be wx-4, there will be a period where 3.x is shipped in parallel as it was while transitioning from 2.x to here. So the current situation with relying on old framework (not even in Debian present anymore) and needing to hope compat keeps on working will be resheduled for 2035+.

@got3nks

got3nks commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

Quick status update — the MIN_WX_VERSION bump to 3.2 is now open as #459. Once it merges, I'll open the follow-up PR that rewrites CHTTPDownloadThread on top of wxWebRequest event-driven on the main thread per option (2) and close this one in its favour.

got3nks added a commit to got3nks/amule that referenced this pull request Apr 23, 2026
Fixes the intermittent startup SIGSEGV tracked in upstream PR amule-project#455.
The crash happens on Linux when a startup HTTP download redirects to
https://, which wxHTTP cannot speak: wxHTTP::Destroy() schedules the
current handler for deferred deletion, the redirect handler throws,
and before the deferred delete runs an epoll event fires on the
still-registered fd — invoking a method on a socket mid-teardown.
Reproducible on stock origin/master (~1-in-27 runs of amuled in a
tight startup loop); the crash-hunt.sh script from PR amule-project#455 passes
>115 iterations clean on Linux/ARM64 with this rewrite. The fault
is inside libwx_baseu_net so it cannot be fixed inside aMule's code
paths — the only reliable remedy is to avoid the wxSocket-based HTTP
path entirely.

The fix is to stop doing HTTP on a worker thread with wxSocket under
the hood. CHTTPDownloadThread now:

* Inherits wxEvtHandler instead of CMuleThread. No worker thread, no
  wxSocket, no wxEpollDispatcher on the download path.

* Owns a wxWebRequest from wxWebSession::GetDefault(), kicks it off
  from the constructor on the main thread, binds wxEVT_WEBREQUEST_STATE
  on the same handler, and lets state transitions drive everything:
  Active → progress event to the optional dialog, Completed → rename
  wx's storage temp file into the caller-supplied path, Failed /
  Cancelled → clean up, Unauthorized → treat as failure. On every
  terminal state the object posts wxEVT_CORE_FINISHED_HTTP_DOWNLOAD
  to the existing dispatcher (CamuleApp::OnFinishedHTTPDownload,
  unchanged) and CallAfter()s its own destruction, so wxWebRequest's
  internal socket is already torn down by the time the handler dies.

* Keeps Create() / Run() / Stop() / OnExit() / Entry() as stubs, so
  the six call sites (new + Create + Run) compile unchanged. When
  wx 3.3's wxWebRequestSync becomes a reasonable floor on distro
  defaults those stubs can be made real and the event-driven
  plumbing collapsed into a synchronous call on a worker thread with
  no wxSocket anywhere.

Reentrancy: no nested event loops anywhere on the fix path. No
ShowModal (the existing progress dialog has always been modeless).
No blocking wait on the request. Cancellation via the dialog's
Cancel button or the app-wide StopAll() is fire-and-forget — it
calls Cancel() on the request, returns immediately, and State_Cancelled
arrives later on the main loop. Avoids the class of bug that would
come from a "sync wrapper = nested wxEventLoop" trick on wx 3.2.

Redirects (including HTTP→HTTPS) are handled by wxWebRequest
transparently, which removes the ~110-line recursive GetInputStream
redirect handler — the actual seat of the upstream race.

Proxy: a static ApplyProxyToDefaultSession() helper maps the existing
CProxyData prefs onto wxWebSession::SetProxy, applied from the ctor.
wxWebProxy / SetProxy are wx 3.3+ only, so the body is guarded with
wxCHECK_VERSION(3,3,0); on wx 3.2 the libcurl backend still honours
http_proxy / https_proxy / all_proxy env vars, so env-driven proxy
keeps working. The legacy wxHTTP path never honoured aMule's Proxy
prefs either (it only called SetProxyMode(bool) with no URL ever
registered via wxURL::SetProxy / SetDefaultProxy anywhere in the
codebase), so this is not a regression. SOCKS is intentionally
skipped even on 3.3+ — wxWebProxy is HTTP-only. A libcurl-direct
backend would be needed if SOCKS support matters, out of scope here.

macOS App Transport Security opt-out
------------------------------------

wxWebRequest on macOS is backed by NSURLSession, which enforces ATS
and blocks plaintext http:// by default. The legacy wxHTTP/wxSocket
path bypassed ATS because it used raw BSD sockets; the rewrite
inherits NSURLSession's policy instead. Without the opt-out every
startup download to an http:// URL fails at the OS level —
reproducible on the first server.met update:

    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.

Inject NSAppTransportSecurity:NSAllowsArbitraryLoads=true via plutil
as a POST_BUILD step on the `amule` target in src/CMakeLists.txt.
Matches the legacy path's behaviour, not a new policy choice.

macOS exit-time crash work-around
---------------------------------

wx 3.3.2 has a bug in wxWebSessionURLSession::~wxWebSessionURLSession:
it releases the NSURLSession and the delegate separately without
first calling -invalidateAndCancel. NSURLSession retains its delegate
strongly, so the session's dealloc already drops the delegate ref —
wx's subsequent release hits a freed object and the process aborts
with "pointer being freed was not allocated". This fires in wx module
cleanup / atexit / __cxa_finalize on any Mac build after any HTTP
download (version check, server.met, …).

By the end of CamuleApp::OnExit we have saved state, joined threads,
and flushed logs — nothing aMule-owned remains to clean up. std::_Exit
bypasses atexit and static destructors, so the buggy wx dtor never
runs and the process terminates cleanly. Linux / Windows continue
through wx's normal cleanup; this macOS-only block can be dropped
once the upstream wx fix lands in a release we depend on.

Dialog / thread lifetime fix
----------------------------

CHTTPDownloadDialog holds a raw pointer (m_owner) to the owning
CHTTPDownloadThread. On successful download the ordered flow is:

  FinishAndDestroy posts wxEVT_HTTP_SHUTDOWN to the dialog, posts
  wxEVT_CORE_FINISHED_HTTP_DOWNLOAD to the app dispatcher, then
  CallAfter()s `delete this`.
  Main loop: dialog OnShutdown → Destroy() (queues the dialog in
  wxPendingDelete), then app dispatcher runs, then CallAfter runs
  and deletes the thread. Later, idle → DeletePendingObjects runs
  the dialog dtor — which used to dereference the now-freed m_owner
  via StopDownload → ~vtable lookup → crash.

Null the dialog's m_owner inside OnShutdown, so its dtor (which runs
later from wxPendingDelete) no longer touches the already-deleted
thread. Equivalent cleanup on the reverse direction (dialog closed
before download completes) was already handled via
DetachCompanion + Stop in StopDownload.

Scope
-----

No CMake build-flag changes. wxWebRequest is in wxBase_net since
wx 3.2.0 (our floor since PR amule-project#459).

* src/HTTPDownload.{h,cpp} — class becomes wxEvtHandler, legacy
  wxThread-style method stubs with a wx 3.3 / wxWebRequestSync TODO,
  DetachCompanion() on the owning side + m_owner nulling on the
  dialog side to make the handshake safe from both directions.
* src/amule.cpp — macOS-only std::_Exit at end of OnExit.
* src/CMakeLists.txt — macOS-only POST_BUILD plutil to inject ATS
  opt-out into the CMake-generated Info.plist.
* Six call sites (version check, server.met, auto-update, IP filter,
  nodes.dat, GeoIP) compile unchanged.
got3nks added a commit to got3nks/amule that referenced this pull request Apr 23, 2026
Fixes the intermittent startup SIGSEGV tracked in upstream PR amule-project#455.
The crash happens on Linux when a startup HTTP download redirects to
https://, which wxHTTP cannot speak: wxHTTP::Destroy() schedules the
current handler for deferred deletion, the redirect handler throws,
and before the deferred delete runs an epoll event fires on the
still-registered fd — invoking a method on a socket mid-teardown.
Reproducible on stock origin/master (~1-in-27 runs of amuled in a
tight startup loop); the crash-hunt.sh script from PR amule-project#455 passes
>115 iterations clean on Linux/ARM64 with this rewrite. The fault
is inside libwx_baseu_net so it cannot be fixed inside aMule's code
paths — the only reliable remedy is to avoid the wxSocket-based HTTP
path entirely.

The fix is to stop doing HTTP on a worker thread with wxSocket under
the hood. CHTTPDownloadThread now:

* Inherits wxEvtHandler instead of CMuleThread. No worker thread, no
  wxSocket, no wxEpollDispatcher on the download path.

* Owns a wxWebRequest from wxWebSession::GetDefault(), kicks it off
  from the constructor on the main thread, binds wxEVT_WEBREQUEST_STATE
  on the same handler, and lets state transitions drive everything:
  Active → progress event to the optional dialog, Completed → rename
  wx's storage temp file into the caller-supplied path, Failed /
  Cancelled → clean up, Unauthorized → treat as failure. On every
  terminal state the object posts wxEVT_CORE_FINISHED_HTTP_DOWNLOAD
  to the existing dispatcher (CamuleApp::OnFinishedHTTPDownload,
  unchanged) and CallAfter()s its own destruction, so wxWebRequest's
  internal socket is already torn down by the time the handler dies.

* Keeps Create() / Run() / Stop() / OnExit() / Entry() as stubs, so
  the six call sites (new + Create + Run) compile unchanged. When
  wx 3.3's wxWebRequestSync becomes a reasonable floor on distro
  defaults those stubs can be made real and the event-driven
  plumbing collapsed into a synchronous call on a worker thread with
  no wxSocket anywhere.

Reentrancy: no nested event loops anywhere on the fix path. No
ShowModal (the existing progress dialog has always been modeless).
No blocking wait on the request. Cancellation via the dialog's
Cancel button or the app-wide StopAll() is fire-and-forget — it
calls Cancel() on the request, returns immediately, and State_Cancelled
arrives later on the main loop. Avoids the class of bug that would
come from a "sync wrapper = nested wxEventLoop" trick on wx 3.2.

Redirects (including HTTP→HTTPS) are handled by wxWebRequest
transparently, which removes the ~110-line recursive GetInputStream
redirect handler — the actual seat of the upstream race.

Proxy: a static ApplyProxyToDefaultSession() helper maps the existing
CProxyData prefs onto wxWebSession::SetProxy, applied from the ctor.
wxWebProxy / SetProxy are wx 3.3+ only, so the body is guarded with
wxCHECK_VERSION(3,3,0); on wx 3.2 the libcurl backend still honours
http_proxy / https_proxy / all_proxy env vars, so env-driven proxy
keeps working. The legacy wxHTTP path never honoured aMule's Proxy
prefs either (it only called SetProxyMode(bool) with no URL ever
registered via wxURL::SetProxy / SetDefaultProxy anywhere in the
codebase), so this is not a regression. SOCKS is intentionally
skipped even on 3.3+ — wxWebProxy is HTTP-only. A libcurl-direct
backend would be needed if SOCKS support matters, out of scope here.

macOS App Transport Security opt-out
------------------------------------

wxWebRequest on macOS is backed by NSURLSession, which enforces ATS
and blocks plaintext http:// by default. The legacy wxHTTP/wxSocket
path bypassed ATS because it used raw BSD sockets; the rewrite
inherits NSURLSession's policy instead. Without the opt-out every
startup download to an http:// URL fails at the OS level —
reproducible on the first server.met update:

    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.

Inject NSAppTransportSecurity:NSAllowsArbitraryLoads=true via plutil
as a POST_BUILD step on the `amule` target in src/CMakeLists.txt.
Matches the legacy path's behaviour, not a new policy choice.

macOS exit-time crash work-around
---------------------------------

wx 3.3.2 has a bug in wxWebSessionURLSession::~wxWebSessionURLSession:
it releases the NSURLSession and the delegate separately without
first calling -invalidateAndCancel. NSURLSession retains its delegate
strongly, so the session's dealloc already drops the delegate ref —
wx's subsequent release hits a freed object and the process aborts
with "pointer being freed was not allocated". This fires in wx module
cleanup / atexit / __cxa_finalize on any Mac build after any HTTP
download (version check, server.met, …).

By the end of CamuleApp::OnExit we have saved state, joined threads,
and flushed logs — nothing aMule-owned remains to clean up. std::_Exit
bypasses atexit and static destructors, so the buggy wx dtor never
runs and the process terminates cleanly. Linux / Windows continue
through wx's normal cleanup; this macOS-only block can be dropped
once the upstream wx fix lands in a release we depend on.

Dialog / thread lifetime fix
----------------------------

CHTTPDownloadDialog holds a raw pointer (m_owner) to the owning
CHTTPDownloadThread. On successful download the ordered flow is:

  FinishAndDestroy posts wxEVT_HTTP_SHUTDOWN to the dialog, posts
  wxEVT_CORE_FINISHED_HTTP_DOWNLOAD to the app dispatcher, then
  CallAfter()s `delete this`.
  Main loop: dialog OnShutdown → Destroy() (queues the dialog in
  wxPendingDelete), then app dispatcher runs, then CallAfter runs
  and deletes the thread. Later, idle → DeletePendingObjects runs
  the dialog dtor — which used to dereference the now-freed m_owner
  via StopDownload → ~vtable lookup → crash.

Null the dialog's m_owner inside OnShutdown, so its dtor (which runs
later from wxPendingDelete) no longer touches the already-deleted
thread. Equivalent cleanup on the reverse direction (dialog closed
before download completes) was already handled via
DetachCompanion + Stop in StopDownload.

Scope
-----

No CMake build-flag changes. wxWebRequest is in wxBase_net since
wx 3.2.0 (our floor since PR amule-project#459).

* src/HTTPDownload.{h,cpp} — class becomes wxEvtHandler, legacy
  wxThread-style method stubs with a wx 3.3 / wxWebRequestSync TODO,
  DetachCompanion() on the owning side + m_owner nulling on the
  dialog side to make the handshake safe from both directions.
* src/amule.cpp — macOS-only std::_Exit at end of OnExit.
* src/CMakeLists.txt — macOS-only POST_BUILD plutil to inject ATS
  opt-out into the CMake-generated Info.plist.
* Six call sites (version check, server.met, auto-update, IP filter,
  nodes.dat, GeoIP) compile unchanged.
got3nks added a commit to got3nks/amule that referenced this pull request Apr 23, 2026
Fixes the intermittent startup SIGSEGV tracked in upstream PR amule-project#455.
The crash happens on Linux when a startup HTTP download redirects to
https://, which wxHTTP cannot speak: wxHTTP::Destroy() schedules the
current handler for deferred deletion, the redirect handler throws,
and before the deferred delete runs an epoll event fires on the
still-registered fd — invoking a method on a socket mid-teardown.
Reproducible on stock origin/master (~1-in-27 runs of amuled in a
tight startup loop); the crash-hunt.sh script from PR amule-project#455 passes
>115 iterations clean on Linux/ARM64 with this rewrite. The fault
is inside libwx_baseu_net so it cannot be fixed inside aMule's code
paths — the only reliable remedy is to avoid the wxSocket-based HTTP
path entirely.

The fix is to stop doing HTTP on a worker thread with wxSocket under
the hood. CHTTPDownloadThread now:

* Inherits wxEvtHandler instead of CMuleThread. No worker thread, no
  wxSocket, no wxEpollDispatcher on the download path.

* Owns a wxWebRequest from wxWebSession::GetDefault(), kicks it off
  from the constructor on the main thread, binds wxEVT_WEBREQUEST_STATE
  on the same handler, and lets state transitions drive everything:
  Active → progress event to the optional dialog, Completed → rename
  wx's storage temp file into the caller-supplied path, Failed /
  Cancelled → clean up, Unauthorized → treat as failure. On every
  terminal state the object posts wxEVT_CORE_FINISHED_HTTP_DOWNLOAD
  to the existing dispatcher (CamuleApp::OnFinishedHTTPDownload,
  unchanged) and CallAfter()s its own destruction, so wxWebRequest's
  internal socket is already torn down by the time the handler dies.

* Keeps Create() / Run() / Stop() / OnExit() / Entry() as stubs, so
  the six call sites (new + Create + Run) compile unchanged. When
  wx 3.3's wxWebRequestSync becomes a reasonable floor on distro
  defaults those stubs can be made real and the event-driven
  plumbing collapsed into a synchronous call on a worker thread with
  no wxSocket anywhere.

Reentrancy: no nested event loops anywhere on the fix path. No
ShowModal (the existing progress dialog has always been modeless).
No blocking wait on the request. Cancellation via the dialog's
Cancel button or the app-wide StopAll() is fire-and-forget — it
calls Cancel() on the request, returns immediately, and State_Cancelled
arrives later on the main loop. Avoids the class of bug that would
come from a "sync wrapper = nested wxEventLoop" trick on wx 3.2.

Redirects (including HTTP→HTTPS) are handled by wxWebRequest
transparently, which removes the ~110-line recursive GetInputStream
redirect handler — the actual seat of the upstream race.

Proxy: a static ApplyProxyToDefaultSession() helper maps the existing
CProxyData prefs onto wxWebSession::SetProxy, applied from the ctor.
wxWebProxy / SetProxy are wx 3.3+ only, so the body is guarded with
wxCHECK_VERSION(3,3,0); on wx 3.2 the libcurl backend still honours
http_proxy / https_proxy / all_proxy env vars, so env-driven proxy
keeps working. The legacy wxHTTP path never honoured aMule's Proxy
prefs either (it only called SetProxyMode(bool) with no URL ever
registered via wxURL::SetProxy / SetDefaultProxy anywhere in the
codebase), so this is not a regression. SOCKS is intentionally
skipped even on 3.3+ — wxWebProxy is HTTP-only. A libcurl-direct
backend would be needed if SOCKS support matters, out of scope here.

macOS App Transport Security opt-out
------------------------------------

wxWebRequest on macOS is backed by NSURLSession, which enforces ATS
and blocks plaintext http:// by default. The legacy wxHTTP/wxSocket
path bypassed ATS because it used raw BSD sockets; the rewrite
inherits NSURLSession's policy instead. Without the opt-out every
startup download to an http:// URL fails at the OS level —
reproducible on the first server.met update:

    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.

Inject NSAppTransportSecurity:NSAllowsArbitraryLoads=true via plutil
as a POST_BUILD step on the `amule` target in src/CMakeLists.txt.
Matches the legacy path's behaviour, not a new policy choice.

macOS exit-time crash work-around
---------------------------------

wx 3.3.2 has a bug in wxWebSessionURLSession::~wxWebSessionURLSession:
it releases the NSURLSession and the delegate separately without
first calling -invalidateAndCancel. NSURLSession retains its delegate
strongly, so the session's dealloc already drops the delegate ref —
wx's subsequent release hits a freed object and the process aborts
with "pointer being freed was not allocated". This fires in wx module
cleanup / atexit / __cxa_finalize on any Mac build after any HTTP
download (version check, server.met, …).

By the end of CamuleApp::OnExit we have saved state, joined threads,
and flushed logs — nothing aMule-owned remains to clean up. std::_Exit
bypasses atexit and static destructors, so the buggy wx dtor never
runs and the process terminates cleanly. Linux / Windows continue
through wx's normal cleanup; this macOS-only block can be dropped
once the upstream wx fix lands in a release we depend on.

Dialog / thread lifetime fix
----------------------------

CHTTPDownloadDialog holds a raw pointer (m_owner) to the owning
CHTTPDownloadThread. On successful download the ordered flow is:

  FinishAndDestroy posts wxEVT_HTTP_SHUTDOWN to the dialog, posts
  wxEVT_CORE_FINISHED_HTTP_DOWNLOAD to the app dispatcher, then
  CallAfter()s `delete this`.
  Main loop: dialog OnShutdown → Destroy() (queues the dialog in
  wxPendingDelete), then app dispatcher runs, then CallAfter runs
  and deletes the thread. Later, idle → DeletePendingObjects runs
  the dialog dtor — which used to dereference the now-freed m_owner
  via StopDownload → ~vtable lookup → crash.

Null the dialog's m_owner inside OnShutdown, so its dtor (which runs
later from wxPendingDelete) no longer touches the already-deleted
thread. Equivalent cleanup on the reverse direction (dialog closed
before download completes) was already handled via
DetachCompanion + Stop in StopDownload.

Scope
-----

No CMake build-flag changes. wxWebRequest is in wxBase_net since
wx 3.2.0 (our floor since PR amule-project#459).

* src/HTTPDownload.{h,cpp} — class becomes wxEvtHandler, legacy
  wxThread-style method stubs with a wx 3.3 / wxWebRequestSync TODO,
  DetachCompanion() on the owning side + m_owner nulling on the
  dialog side to make the handshake safe from both directions.
* src/amule.cpp — macOS-only std::_Exit at end of OnExit.
* src/CMakeLists.txt — macOS-only POST_BUILD plutil to inject ATS
  opt-out into the CMake-generated Info.plist.
* Six call sites (version check, server.met, auto-update, IP filter,
  nodes.dat, GeoIP) compile unchanged.
got3nks added a commit to got3nks/amule that referenced this pull request Apr 23, 2026
Fixes the intermittent startup SIGSEGV tracked in upstream PR amule-project#455.
The crash happens on Linux when a startup HTTP download redirects to
https://, which wxHTTP cannot speak: wxHTTP::Destroy() schedules the
current handler for deferred deletion, the redirect handler throws,
and before the deferred delete runs an epoll event fires on the
still-registered fd — invoking a method on a socket mid-teardown.
Reproducible on stock origin/master (~1-in-27 runs of amuled in a
tight startup loop); the crash-hunt.sh script from PR amule-project#455 passes
>115 iterations clean on Linux/ARM64 with this rewrite. The fault
is inside libwx_baseu_net so it cannot be fixed inside aMule's code
paths — the only reliable remedy is to avoid the wxSocket-based HTTP
path entirely.

The fix is to stop doing HTTP on a worker thread with wxSocket under
the hood. CHTTPDownloadThread now:

* Inherits wxEvtHandler instead of CMuleThread. No worker thread, no
  wxSocket, no wxEpollDispatcher on the download path.

* Owns a wxWebRequest from wxWebSession::GetDefault(), kicks it off
  from the constructor on the main thread, binds wxEVT_WEBREQUEST_STATE
  on the same handler, and lets state transitions drive everything:
  Active → progress event to the optional dialog, Completed → rename
  wx's storage temp file into the caller-supplied path, Failed /
  Cancelled → clean up, Unauthorized → treat as failure. On every
  terminal state the object posts wxEVT_CORE_FINISHED_HTTP_DOWNLOAD
  to the existing dispatcher (CamuleApp::OnFinishedHTTPDownload,
  unchanged) and CallAfter()s its own destruction, so wxWebRequest's
  internal socket is already torn down by the time the handler dies.

* Keeps Create() / Run() / Stop() / OnExit() / Entry() as stubs, so
  the six call sites (new + Create + Run) compile unchanged. When
  wx 3.3's wxWebRequestSync becomes a reasonable floor on distro
  defaults those stubs can be made real and the event-driven
  plumbing collapsed into a synchronous call on a worker thread with
  no wxSocket anywhere.

Reentrancy: no nested event loops anywhere on the fix path. No
ShowModal (the existing progress dialog has always been modeless).
No blocking wait on the request. Cancellation via the dialog's
Cancel button or the app-wide StopAll() is fire-and-forget — it
calls Cancel() on the request, returns immediately, and State_Cancelled
arrives later on the main loop. Avoids the class of bug that would
come from a "sync wrapper = nested wxEventLoop" trick on wx 3.2.

Redirects (including HTTP→HTTPS) are handled by wxWebRequest
transparently, which removes the ~110-line recursive GetInputStream
redirect handler — the actual seat of the upstream race.

Proxy: a static ApplyProxyToDefaultSession() helper maps the existing
CProxyData prefs onto wxWebSession::SetProxy, applied from the ctor.
wxWebProxy / SetProxy are wx 3.3+ only, so the body is guarded with
wxCHECK_VERSION(3,3,0); on wx 3.2 the libcurl backend still honours
http_proxy / https_proxy / all_proxy env vars, so env-driven proxy
keeps working. The legacy wxHTTP path never honoured aMule's Proxy
prefs either (it only called SetProxyMode(bool) with no URL ever
registered via wxURL::SetProxy / SetDefaultProxy anywhere in the
codebase), so this is not a regression. SOCKS is intentionally
skipped even on 3.3+ — wxWebProxy is HTTP-only. A libcurl-direct
backend would be needed if SOCKS support matters, out of scope here.

macOS App Transport Security opt-out
------------------------------------

wxWebRequest on macOS is backed by NSURLSession, which enforces ATS
and blocks plaintext http:// by default. The legacy wxHTTP/wxSocket
path bypassed ATS because it used raw BSD sockets; the rewrite
inherits NSURLSession's policy instead. Without the opt-out every
startup download to an http:// URL fails at the OS level —
reproducible on the first server.met update:

    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.

Inject NSAppTransportSecurity:NSAllowsArbitraryLoads=true via plutil
as a POST_BUILD step on the `amule` target in src/CMakeLists.txt.
Matches the legacy path's behaviour, not a new policy choice.

macOS exit-time crash work-around
---------------------------------

wx 3.3.2 has a bug in wxWebSessionURLSession::~wxWebSessionURLSession:
it releases the NSURLSession and the delegate separately without
first calling -invalidateAndCancel. NSURLSession retains its delegate
strongly, so the session's dealloc already drops the delegate ref —
wx's subsequent release hits a freed object and the process aborts
with "pointer being freed was not allocated". This fires in wx module
cleanup / atexit / __cxa_finalize on any Mac build after any HTTP
download (version check, server.met, …).

By the end of CamuleApp::OnExit we have saved state, joined threads,
and flushed logs — nothing aMule-owned remains to clean up. std::_Exit
bypasses atexit and static destructors, so the buggy wx dtor never
runs and the process terminates cleanly. Linux / Windows continue
through wx's normal cleanup; this macOS-only block can be dropped
once the upstream wx fix lands in a release we depend on.

Windows portable install: CA bundle for libcurl HTTPS
-----------------------------------------------------

wxWebRequest on MSYS2 (MINGW64 / CLANGARM64) is backed by libcurl
built with OpenSSL. MSYS2 libcurl is compiled with an absolute
--with-ca-bundle= path (e.g. /clangarm64/etc/ssl/certs/ca-bundle.crt)
that only exists on the developer's machine. On an end-user box the
CA bundle isn't there and every HTTPS request (including the HTTP→
HTTPS redirect that hits the default startup URLs) fails with
"libcurl error 77: Problem with the SSL CA cert".

Two-part fix so portable zips work everywhere:

* src/CMakeLists.txt (WIN32 AND MINGW install block): ship the MSYS2
  CA bundle alongside the bundled DLLs as bin/ca-bundle.crt.
* src/amule.cpp (CamuleApp::OnInit, __WINDOWS__-guarded): if
  CURL_CA_BUNDLE is not already set in the environment, point it at
  the bundled file resolved from wxStandardPaths::GetExecutablePath.

Users running from a portable directory now get working HTTPS without
any shell env setup; users who want to point at their own bundle
still can by setting CURL_CA_BUNDLE before launch.

Dialog / thread lifetime fix
----------------------------

CHTTPDownloadDialog holds a raw pointer (m_owner) to the owning
CHTTPDownloadThread. On successful download the ordered flow is:

  FinishAndDestroy posts wxEVT_HTTP_SHUTDOWN to the dialog, posts
  wxEVT_CORE_FINISHED_HTTP_DOWNLOAD to the app dispatcher, then
  CallAfter()s `delete this`.
  Main loop: dialog OnShutdown → Destroy() (queues the dialog in
  wxPendingDelete), then app dispatcher runs, then CallAfter runs
  and deletes the thread. Later, idle → DeletePendingObjects runs
  the dialog dtor — which used to dereference the now-freed m_owner
  via StopDownload → ~vtable lookup → crash.

Null the dialog's m_owner inside OnShutdown, so its dtor (which runs
later from wxPendingDelete) no longer touches the already-deleted
thread. Equivalent cleanup on the reverse direction (dialog closed
before download completes) was already handled via
DetachCompanion + Stop in StopDownload.

Progress bar safety (wxGTK)
---------------------------

wxWebRequest::GetBytesExpectedToReceive() returns wxInvalidOffset (-1)
before Content-Length is known (or indefinitely if the server never
sends it). Forwarding -1 into CHTTPDownloadDialog::UpdateGauge ended
up calling wxGauge::SetRange(-1), which trips an assertion on the
next repaint on wxGTK (./src/gtk/gauge.cpp:90, reproducible on IP
filter updates on Ubuntu). Clamp the expected size at the event
source, and make UpdateGauge only touch the gauge when the total is
known and current <= total, so out-of-range values can never reach
the widget.

Scope
-----

wxWebRequest is in wxBase_net since wx 3.2.0 (our floor since PR amule-project#459),
so no new build dependency beyond install-time packaging of an MSYS2
CA bundle on Windows.

* src/HTTPDownload.{h,cpp} — class becomes wxEvtHandler, legacy
  wxThread-style method stubs with a wx 3.3 / wxWebRequestSync TODO,
  DetachCompanion() on the owning side + m_owner nulling on the
  dialog side to make the handshake safe from both directions,
  UpdateGauge clamped.
* src/amule.cpp — macOS-only std::_Exit at end of OnExit;
  Windows-only CURL_CA_BUNDLE fallback to the bundled ca-bundle.crt.
* src/CMakeLists.txt — macOS-only POST_BUILD plutil to inject ATS
  opt-out into the CMake-generated Info.plist; Windows-only install
  of the MSYS2 CA bundle alongside the bundled DLLs.
* Six call sites (version check, server.met, auto-update, IP filter,
  nodes.dat, GeoIP) compile unchanged.
@got3nks

got3nks commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #462, which fixes the same startup SIGSEGV but via a different approach: rewriting CHTTPDownloadThread on top of wxWebRequest instead of linking an optional libcurl backend.

Reasons the wxWebRequest approach ended up being a better fit:

Closing in favour of #462.

@got3nks got3nks closed this Apr 23, 2026
mrjimenez pushed a commit that referenced this pull request Apr 23, 2026
Fixes the intermittent startup SIGSEGV tracked in upstream PR #455.
The crash happens on Linux when a startup HTTP download redirects to
https://, which wxHTTP cannot speak: wxHTTP::Destroy() schedules the
current handler for deferred deletion, the redirect handler throws,
and before the deferred delete runs an epoll event fires on the
still-registered fd — invoking a method on a socket mid-teardown.
Reproducible on stock origin/master (~1-in-27 runs of amuled in a
tight startup loop); the crash-hunt.sh script from PR #455 passes
>115 iterations clean on Linux/ARM64 with this rewrite. The fault
is inside libwx_baseu_net so it cannot be fixed inside aMule's code
paths — the only reliable remedy is to avoid the wxSocket-based HTTP
path entirely.

The fix is to stop doing HTTP on a worker thread with wxSocket under
the hood. CHTTPDownloadThread now:

* Inherits wxEvtHandler instead of CMuleThread. No worker thread, no
  wxSocket, no wxEpollDispatcher on the download path.

* Owns a wxWebRequest from wxWebSession::GetDefault(), kicks it off
  from the constructor on the main thread, binds wxEVT_WEBREQUEST_STATE
  on the same handler, and lets state transitions drive everything:
  Active → progress event to the optional dialog, Completed → rename
  wx's storage temp file into the caller-supplied path, Failed /
  Cancelled → clean up, Unauthorized → treat as failure. On every
  terminal state the object posts wxEVT_CORE_FINISHED_HTTP_DOWNLOAD
  to the existing dispatcher (CamuleApp::OnFinishedHTTPDownload,
  unchanged) and CallAfter()s its own destruction, so wxWebRequest's
  internal socket is already torn down by the time the handler dies.

* Keeps Create() / Run() / Stop() / OnExit() / Entry() as stubs, so
  the six call sites (new + Create + Run) compile unchanged. When
  wx 3.3's wxWebRequestSync becomes a reasonable floor on distro
  defaults those stubs can be made real and the event-driven
  plumbing collapsed into a synchronous call on a worker thread with
  no wxSocket anywhere.

Reentrancy: no nested event loops anywhere on the fix path. No
ShowModal (the existing progress dialog has always been modeless).
No blocking wait on the request. Cancellation via the dialog's
Cancel button or the app-wide StopAll() is fire-and-forget — it
calls Cancel() on the request, returns immediately, and State_Cancelled
arrives later on the main loop. Avoids the class of bug that would
come from a "sync wrapper = nested wxEventLoop" trick on wx 3.2.

Redirects (including HTTP→HTTPS) are handled by wxWebRequest
transparently, which removes the ~110-line recursive GetInputStream
redirect handler — the actual seat of the upstream race.

Proxy: a static ApplyProxyToDefaultSession() helper maps the existing
CProxyData prefs onto wxWebSession::SetProxy, applied from the ctor.
wxWebProxy / SetProxy are wx 3.3+ only, so the body is guarded with
wxCHECK_VERSION(3,3,0); on wx 3.2 the libcurl backend still honours
http_proxy / https_proxy / all_proxy env vars, so env-driven proxy
keeps working. The legacy wxHTTP path never honoured aMule's Proxy
prefs either (it only called SetProxyMode(bool) with no URL ever
registered via wxURL::SetProxy / SetDefaultProxy anywhere in the
codebase), so this is not a regression. SOCKS is intentionally
skipped even on 3.3+ — wxWebProxy is HTTP-only. A libcurl-direct
backend would be needed if SOCKS support matters, out of scope here.

macOS App Transport Security opt-out
------------------------------------

wxWebRequest on macOS is backed by NSURLSession, which enforces ATS
and blocks plaintext http:// by default. The legacy wxHTTP/wxSocket
path bypassed ATS because it used raw BSD sockets; the rewrite
inherits NSURLSession's policy instead. Without the opt-out every
startup download to an http:// URL fails at the OS level —
reproducible on the first server.met update:

    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.

Inject NSAppTransportSecurity:NSAllowsArbitraryLoads=true via plutil
as a POST_BUILD step on the `amule` target in src/CMakeLists.txt.
Matches the legacy path's behaviour, not a new policy choice.

macOS exit-time crash work-around
---------------------------------

wx 3.3.2 has a bug in wxWebSessionURLSession::~wxWebSessionURLSession:
it releases the NSURLSession and the delegate separately without
first calling -invalidateAndCancel. NSURLSession retains its delegate
strongly, so the session's dealloc already drops the delegate ref —
wx's subsequent release hits a freed object and the process aborts
with "pointer being freed was not allocated". This fires in wx module
cleanup / atexit / __cxa_finalize on any Mac build after any HTTP
download (version check, server.met, …).

By the end of CamuleApp::OnExit we have saved state, joined threads,
and flushed logs — nothing aMule-owned remains to clean up. std::_Exit
bypasses atexit and static destructors, so the buggy wx dtor never
runs and the process terminates cleanly. Linux / Windows continue
through wx's normal cleanup; this macOS-only block can be dropped
once the upstream wx fix lands in a release we depend on.

Windows portable install: CA bundle for libcurl HTTPS
-----------------------------------------------------

wxWebRequest on MSYS2 (MINGW64 / CLANGARM64) is backed by libcurl
built with OpenSSL. MSYS2 libcurl is compiled with an absolute
--with-ca-bundle= path (e.g. /clangarm64/etc/ssl/certs/ca-bundle.crt)
that only exists on the developer's machine. On an end-user box the
CA bundle isn't there and every HTTPS request (including the HTTP→
HTTPS redirect that hits the default startup URLs) fails with
"libcurl error 77: Problem with the SSL CA cert".

Two-part fix so portable zips work everywhere:

* src/CMakeLists.txt (WIN32 AND MINGW install block): ship the MSYS2
  CA bundle alongside the bundled DLLs as bin/ca-bundle.crt.
* src/amule.cpp (CamuleApp::OnInit, __WINDOWS__-guarded): if
  CURL_CA_BUNDLE is not already set in the environment, point it at
  the bundled file resolved from wxStandardPaths::GetExecutablePath.

Users running from a portable directory now get working HTTPS without
any shell env setup; users who want to point at their own bundle
still can by setting CURL_CA_BUNDLE before launch.

Dialog / thread lifetime fix
----------------------------

CHTTPDownloadDialog holds a raw pointer (m_owner) to the owning
CHTTPDownloadThread. On successful download the ordered flow is:

  FinishAndDestroy posts wxEVT_HTTP_SHUTDOWN to the dialog, posts
  wxEVT_CORE_FINISHED_HTTP_DOWNLOAD to the app dispatcher, then
  CallAfter()s `delete this`.
  Main loop: dialog OnShutdown → Destroy() (queues the dialog in
  wxPendingDelete), then app dispatcher runs, then CallAfter runs
  and deletes the thread. Later, idle → DeletePendingObjects runs
  the dialog dtor — which used to dereference the now-freed m_owner
  via StopDownload → ~vtable lookup → crash.

Null the dialog's m_owner inside OnShutdown, so its dtor (which runs
later from wxPendingDelete) no longer touches the already-deleted
thread. Equivalent cleanup on the reverse direction (dialog closed
before download completes) was already handled via
DetachCompanion + Stop in StopDownload.

Progress bar safety (wxGTK)
---------------------------

wxWebRequest::GetBytesExpectedToReceive() returns wxInvalidOffset (-1)
before Content-Length is known (or indefinitely if the server never
sends it). Forwarding -1 into CHTTPDownloadDialog::UpdateGauge ended
up calling wxGauge::SetRange(-1), which trips an assertion on the
next repaint on wxGTK (./src/gtk/gauge.cpp:90, reproducible on IP
filter updates on Ubuntu). Clamp the expected size at the event
source, and make UpdateGauge only touch the gauge when the total is
known and current <= total, so out-of-range values can never reach
the widget.

Scope
-----

wxWebRequest is in wxBase_net since wx 3.2.0 (our floor since PR #459),
so no new build dependency beyond install-time packaging of an MSYS2
CA bundle on Windows.

* src/HTTPDownload.{h,cpp} — class becomes wxEvtHandler, legacy
  wxThread-style method stubs with a wx 3.3 / wxWebRequestSync TODO,
  DetachCompanion() on the owning side + m_owner nulling on the
  dialog side to make the handshake safe from both directions,
  UpdateGauge clamped.
* src/amule.cpp — macOS-only std::_Exit at end of OnExit;
  Windows-only CURL_CA_BUNDLE fallback to the bundled ca-bundle.crt.
* src/CMakeLists.txt — macOS-only POST_BUILD plutil to inject ATS
  opt-out into the CMake-generated Info.plist; Windows-only install
  of the MSYS2 CA bundle alongside the bundled DLLs.
* Six call sites (version check, server.met, auto-update, IP filter,
  nodes.dat, GeoIP) compile unchanged.
@got3nks
got3nks deleted the http-libcurl-pr branch May 3, 2026 15:19
mrjimenez pushed a commit that referenced this pull request May 5, 2026
Following the org-migration discussion on PR #521, point all source /
docs / packaging URLs at the new `amule-org` GitHub org that
@mrjimenez bootstrapped on 2026-05-04.

Two motivating reasons (per the thread):

1. The new active maintainer team needs proper write/admin access
   that the existing amule-project upper-level admins have been
   unreachable to grant.

2. The version-check probe added in #522 hardcodes its target URL
   into every shipped binary.  If that URL stays at amule-project
   while future releases land on amule-org, the cohort of users on
   3.0.0 will be permanently stranded querying a dead-end endpoint
   and never learn about 3.0.1+ via the in-app prompt.  Flipping
   the URL pre-3.0.0-tag is the only way to keep them informed
   without dual-publishing maintenance burden.

Seven references updated in lockstep:

- `src/amule.cpp` — version-check probe URL.
- `packaging/linux/flatpak/org.amule.aMule.yaml.in` — Flatpak
  manifest's git source URL (load-bearing — `flatpak-builder`
  clones from this URL at build time).
- `org.amule.aMule.metainfo.xml` — AppStream bug-tracker URL,
  surfaces in Flathub / GNOME Software / KDE Discover.
- `README.md` — logo image URL on raw.githubusercontent.com plus
  the Issues and Pull Requests link references.
- `docs/INSTALL.md` — upstream-issue-tracker doc reference.
- `docs/README.md` — GitHub Issues doc reference.

Left as-is:

- `src/HTTPDownload.cpp:252` — code comment referencing issue
  `#455` for historical context.  The issue
  itself stays at amule-project regardless of where future
  development happens; the comment is a citation, not a forward
  reference.
- The `amule-project.de` / `amule-project.net` mentions in
  `docs/CHANGELOG.md` 2003-era entries — those are old DNS
  domain references unrelated to the GitHub org.

Note: this commit assumes `amule-org/amule` will exist as a real
repo by the time this PR merges.  The org was created 2026-05-04
with zero repos; the Flatpak build URL change in particular will
fail the Packaging workflow until the repo is bootstrapped.
ngosang pushed a commit to ngosang/amule that referenced this pull request Jul 12, 2026
…-application

Translations update from Weblate
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.

4 participants