Trim fd_set memory, fix timeout math, stop busy-spinning in verify(jwt) - #224
Open
djw8605 wants to merge 1 commit into
Open
Trim fd_set memory, fix timeout math, stop busy-spinning in verify(jwt)#224djw8605 wants to merge 1 commit into
djw8605 wants to merge 1 commit into
Conversation
Three related I/O-loop cleanups: - SimpleCurlGet declared its select() sets as fd_set[FD_SETSIZE] -- arrays of 1024 fd_sets where a single fd_set (which already holds FD_SETSIZE descriptors) was intended. That is roughly 384KB of wasted memory per in-flight request; it only worked because the arrays decayed to a pointer to their first element. - AsyncStatus::get_timeout_val() computed 100 * (seconds remaining) where every other site converts with 1000, making C-API event loops poll 10x more often than intended; it also produced a negative timeval once the deadline passed, which is invalid for select() (EINVAL). Use 1000x and clamp at zero. - The verify(jwt) overload -- the path scitoken_deserialize uses -- looped verify_async_continue() with no select() at all, pinning a CPU core for the full duration of any JWKS fetch. Wait on the transfer's file descriptors (capped at one second, and continuing after timeouts as libcurl requires) instead. Co-Authored-By: Claude Fable 5 <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
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.
Three related I/O-loop cleanups:
1. ~384 KB of fd_sets per in-flight request
SimpleCurlGetdeclared its select() sets asfd_set m_read_fd_set[FD_SETSIZE]— an array of 1024fd_sets (×3 members) where a singlefd_set(which already holdsFD_SETSIZEdescriptors) was intended. Roughly 384 KB of wasted memory perSimpleCurlGet, i.e., per in-flight verification. It only worked because the arrays decayed to a pointer to their first element.2. Timeout math in
AsyncStatus::get_timeout_val()Computed
100 * (seconds remaining)where every other site converts seconds→ms with1000(compareSimpleCurlGet::perform). C-API event loops usingscitoken_status_get_timeout_valtherefore polled 10× more often than intended. It also produced a negativetimevalonce the deadline passed — invalid forselect()(EINVAL). Now1000 *with a clamp at zero.3. Busy-spin in
verify(jwt)The
verify(const jwt::decoded_jwt&)overload — the pathscitoken_deserializeuses — loopedverify_async_continue()with noselect()at all, pinning a CPU core for the full duration of any JWKS fetch over the network. It now waits on the transfer's descriptors (capped at 1 s) and continues unconditionally afterward, as libcurl requires after timeouts.Testing
ctestunit, env_config, and monitoring suites pass (deserialize tests exercise the reworked loop).🤖 Generated with Claude Code