Alan T. DeKok [Mon, 3 Aug 2026 00:00:52 +0000 (20:00 -0400)]
add stack short-cuts for break, continue, return, and load-balance
so that we don't have to walk back up the stack to find them.
Instead, we can just examine the current stack frame to see where
those frames are.
For now. we just set the return frames and assert that their values
are correct. We do not (as yet) skip the loop over the frames,
to just find the correct frame.
During connection establishement, MariaDB Connector/C can close the fd
and potentially open a new one without any callbacks to notify of that
happening.
If we have events registered for the fd that has now been closed,
potentially events fire for that fd when it has been re-used for a
different purpose.
Using dup() to create our own fd for the socket, we remain in control of
the fd which has registered events.
listen: disarm the timers when closing a virtual listener
proto_load_step and proto_cron_crontab drive themselves from timers rather than from their fd, and neither had a close callback, so nothing stopped them when the listener was closed. The load generator kept manufacturing packets and handing them to fr_network_send_request() after the network had signalled the workers to close, which the assert added there caught.
Both now close the fd and remove their timer. Of the remaining app_io modules with timers, proto_detail_file, proto_detail_work and proto_ldap_sync_ldap have a close callback that does not obviously disarm anything, and are worth a look.
53bfbf591b updated configure.ac from valgrind.h to valgrind/valgrind.h, but hand-edited the generated configure rather than regenerating it, and typo'd the path as "valgind/valgrind.h". The cache variable name is spelled correctly, so only the check itself was broken.
HAVE_VALGRIND_VALGRIND_H has therefore been undefined since, which means RUNNING_ON_VALGRIND falls back to the 0 stub: dl.c dlcloses modules even under valgrind, losing the symbols of unloaded code, and radiusd -Xxv reports runtime-valgrind as false while running under it.
io: stop queueing requests to workers once the close has been signalled
worker.c asserts that the network adds nothing to a channel after FR_CHANNEL_CLOSE, but nothing enforced it. A packet queued between the signal and the worker acking is discarded by the worker without a reply, so the network never decrements the socket's outstanding count for it, and the socket can then never be freed.
nr->exiting is now set before the workers are signalled rather than after, and fr_network_send_request() - the single funnel all three senders go through - asserts it is clear. The listeners are closed before the signal, so a socket read cannot reach there; anything that does is a path worth seeing a backtrace for, and release builds still just drop the packet.
io: hand back requests still queued when a responder closes its channel
The worker cancelled the requests it had already popped, but anything still sitting in the channel's atomic queue was abandoned: after the ack the channel is gone, so those messages were never marked done, and the ring buffer they came from could not be reclaimed.
fr_channel_responder_discard() drains what is left and marks it done, called just before the ack. It is a safety net rather than a fix, and warns when it finds anything: the network is not supposed to enqueue once it has signalled the close, which is what worker.c's own "Network added messages to channel after sending FR_CHANNEL_CLOSE" assert claims, and a discarded request produces no reply, so the network never decrements its outstanding count for it and the socket cannot be freed regardless.
io: free the network sockets only once the workers have acked the close
A socket's message set backs every request queued to a worker, so freeing the socket while a worker still holds one leaves it reading poisoned memory: it faults in worker_request_bootstrap() on a cd->listen of 0x4242424242424242, which is non-canonical and so presents as a GPF rather than an ordinary SIGSEGV. fr_network_destroy() now closes the sockets and leaves them allocated, and fr_network() frees them after its loop, which already runs until every worker has acked for the same reason.
The reply drain stays where it was, before the workers are signalled, because that side of the ordering is the mirror image: replies live in the workers' message sets, which they free as they exit, so we have to be finished with those first.
io: separate closing a network socket from freeing it
Freeing a socket frees s->ms, but the workers hold messages allocated from it until they ack the channel close, so the descriptor and the memory cannot share a lifetime. network_socket_close() takes the event removal and the close, the destructor asserts it has already run, and the socket keeps the handle fr_event_fd_insert() gave it rather than tracking whether it has one.
The event is now parented off the socket rather than the network, so it cannot outlive the socket that owns the handle even if a path forgets to close. fr_network_socket_dead() closes up front, since nothing writes to a dead socket, and the remaining free sites close immediately beforehand, so the descriptor is released no later than before. fr_network_destroy() is still wrong - it frees the sockets before signalling the workers at all - but it no longer zeroes s->outstanding to hide it.
ci: gate the automatic merge on the DEB, RPM and sanitizer legs too
The merge ran as a job at the end of CI with needs: ci, which only waited for CI itself because needs: cannot reach jobs in another workflow. A red DEB, RPM or sanitizer leg did not stop a developer branch reaching master.
Moved to its own workflow_run workflow instead. That fires once per gating workflow completion rather than once when they have all finished, so it runs several times per commit and does nothing until the last leg lands, which avoids a job sitting in a poll loop holding a runner. It now merges the commit the workflows ran on rather than the branch ref, so a branch that moved on since cannot put an unvalidated commit onto master.
connection: expose is_closed so a closed connection's handle can't be used
The close callback frees the handle, but is_closed lived in the private struct so drivers had no way to tell a live connection from a dead one. A mysql connection dying mid-query aborted on talloc_get_type_abort() in sql_error(), reached from sql_get_map_list_resume() after the trunk had already closed the connection.
rlm_sql_print_error() is the only caller of driver->sql_error, and every driver reads the errors off the handle, so the check goes there once and reports "Connection closed" rather than in each driver.
Alan T. DeKok [Sun, 26 Jul 2026 15:06:38 +0000 (11:06 -0400)]
determine dictionary for virtual modules
by walking through the entire block, not just the top-level ones.
this makes the function more robust for things like redundant
containing load-balance.
multi-server: never publish profiling results from an unclean valgrind exit
Run 30180044334 published results from two suites where valgrind had killed
FreeRADIUS partway through. Callgrind output from a killed run is truncated at
whatever point it died, so the numbers are not comparable with a clean run, and
publishing them poisons the per-suite history the regression gate compares
against.
start_valgrind_profiling.sh now records valgrind's exit status in
valgrind-exit-status alongside the rest of a test's results, and says loudly
when the status means a signal. The status is written for clean runs too, so an
absent file means the wrapper never got that far rather than that the run was
fine.
publish-profiling-results.sh refuses to publish when any test's status is
non-zero, naming each suite and status, and exits non-zero so the leg goes red
rather than passing having uploaded nothing. Trees with no status file at all
publish as before.
The status file is small and travels with the results, so the store keeps the
provenance of every published run.
ci: collect core dumps as artifacts, keep them out of the profiling publish
Most suites run with allow_core_dumps = yes, but nothing in CI ever collected
a core, so the only record of a crash the logs cannot reconstruct was thrown
away with the runner.
scripts/ci/collect-core-dumps.sh stages cores into ci-core-dumps/, preserving
each path below the searched root so the suite that crashed stays identifiable.
The new .github/actions/collect-core-dumps composite action calls the script and
uploads the result, and ci, ci-sanitizers, ci-macos, ci-deb, ci-rpm and
ci-multi-server-tests all use it. Collection runs on success as well as failure:
FreeRADIUS crashing during shutdown does not necessarily fail the suite that
provoked the crash, which is how run 30180044334 reported success while dumping
two 1.5G cores.
publish-profiling-results.sh now drops cores from the store tarball. Those two
cores turned a 6M publish into 35M of mostly-zero pages, the store's synchronous
ingest then ran ~64s, and wproxy's 60s proxy_read_timeout returned a 504 for a
publish that had in fact landed. The multi-server leg passes move: true so a
core leaves prof-results/ and rides one artifact rather than inflating the
prof-results artifact too. Exclusions are always logged with their sizes; a
silently shrunk publish would be worse than a noisy one.
The shared name patterns live in scripts/ci/core-dump-names.inc so the script
that keeps cores and the script that drops them cannot disagree.
Not covered: ci-freebsd, whose VM syncs runner-to-vm only, and the fuzzing leg,
which has its own crash artifacts. Whether a core is written at all still
depends on the ambient RLIMIT_CORE and kernel.core_pattern, neither of which CI
sets; allow_core_dumps only restores the limit FreeRADIUS inherited.
build: categorise library targets via TGT_CATEGORY
Library-producing makefile fragments declare a category next to
TARGET: lib-protocol for the wire codecs under src/protocols, and
lib-util for the other support libraries. Modules, listeners and
binaries stay uncategorised for now.
One declaration per fragment, placed after the last TARGET
assignment or probe include and outside any conditional, so the
fragment's own value wins over one dragged in by textually including
another fragment (the configure-probe pattern used by rlm_json and
friends). Fragments which probe but aren't libraries keep a bare
'TGT_CATEGORY :=' reset for the same reason. Configure-generated
fragments carry the declaration in their all.mk.in template.
boiler.mk only gains generic plumbing: the TGT_CATEGORY passthrough
and a CATEGORY_TARGETS macro which expands to all targets in the
given categories.
'make library.list' prints '<category> <name>' pairs - the first
authoritative list of the libraries the build produces, usable to
keep the debian/redhat packaging manifests honest. Census:
14 lib-protocol, 18 lib-util.
src/stats/all.mk now derives the radict chain-load dependencies from
CATEGORY_TARGETS instead of name-prefix filtering ALL_TGTS.
build: stats generation depends on the dhcpv4/dhcpv6 libraries
radict chain-loads the DHCPv4 and DHCPv6 dictionary validation
libraries while parsing the RADIUS dictionaries (dictionary.rfc9445
has ref=@DHCPv4 / ref=@DHCPv6), but the DICT_STATS rules only
depended on libfreeradius-radius and radict itself, so a parallel
build could run radict before those libraries were linked and fail
with "cannot open shared object file", as seen intermittently on the
rocky10 service image build.
radiusd: only SIGTERM the process group when we lead it
The shutdown group-wide SIGTERM exists to reap children left over
from exec calls and triggers. When radiusd is daemonized (setsid),
or given its own group by systemd or a container runtime, the group
contains only radiusd and its children and the signal is safe. Run
in the foreground from a shell, the group belongs to the shell, and
the signal killed the shell and its terminal too.
multi-server: drive proto_load with max_requests / on_complete
The listen configs previously set 'num_messages' inside step { },
which nothing parsed, and relied on the wall-clock ramp to produce
the expected packet count - a few skipped timer slots on a loaded CI
runner meant fewer packets than the consumers expected, and flaky
kafka-produce failures.
Point the new max_requests knob at the per-test count and rename the
env var TEST_LOADGEN_NUM_MESSAGES to TEST_LOADGEN_MAX_REQUESTS to
match. on_complete = stop keeps the servers running for the rest of
each test.
projects / thirdparty / freeradius-server.git / log
