Fix client hang on req-id collision and no_ports port-wrap crash - #248
Merged
Conversation
- .github/workflows/main.yml: artifact path was .tar.xz but the tar
command produces .tar.zst; upload would always silently fail
- src/eradius_eap_packet.erl: fix doc typos 'endecode a EPA' -> 'encode an EAP'
- src/eradius_dict.erl: code_change returned {ok, state} (atom) instead
of {ok, State} (variable), corrupting server state on hot code upgrade
- src/eradius_client_socket.erl: handle_call catch-all returned {noreply}
which would hang callers; use {reply, {error, bad_call}, State}
- src/eradius.app.src: capitalize 'Github' -> 'GitHub'
- src/eradius_log.erl: line/1 and format_sender/1 always used server_addr
but the docstring says '<Client-IP>'; add peer_addr/1 helper that picks
client_addr (peer/NAS on server side) and falls back to server_addr
(remote server on client side)
- test/eradius_client_SUITE.erl: fix typo 'Adress' -> 'Address'
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
(NumberOfPorts - 1) skipped the last port and crashed with badarith (rem 0) when no_ports = 1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gen_server:call used infinity; if a pending request entry was
overwritten by a same-ReqId request the caller waited forever. Use
Timeout + margin and map exit:{timeout,_} to {error,timeout}. Reply to
unknown calls instead of leaving them unanswered.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
eradius_client_socket:send_request/5call timeout (wasinfinity) so a caller whose pending entry is overwritten by a same-ReqId request returns{error, timeout}instead of hanging forever. UsesTimeout + ?CALL_TIMEOUT_MARGINand mapsexit:{timeout,_}to{error, timeout};infinityis passed through unchanged.{error, unknown_request}to unexpectedhandle_callin both the client socket and the client manager (the manager previously returned{noreply, State}, which would now time out instead of erroring fast).next_port_and_req_id/3:rem NumberOfPorts(wasNumberOfPorts - 1) — fixes the off-by-one that skipped the last port and thebadarith(rem 0) crash whenno_ports = 1.Test Plan
no_ports_one_wraps— 257 allocations force the req-id wrap branch withno_ports = 1; fails withbadarithbefore the fix, passes after.clobber_does_not_hang— a pending request whose entry is clobbered by a same-ReqId request returns{error, timeout}instead of hanging; fails ("P1 hung") before the fix, passes after.rebar3 ctgreen on OTP 28.3 — all 111 tests pass (both new cases run acrossipv4,ipv4_mapped_ipv6,ipv6).rebar3 fmtclean.Deferred: safe ReqId reuse (duplicate-detection-aware allocation with time-based quarantine) — the structural root cause — is intentionally out of scope and tracked separately.
🤖 Generated with Claude Code