build std's shared lookup tables at module load, not on first use - #624
Merged
Conversation
the sha round constants, the hpack static table and the huffman decode map were each filled in on first use behind an unsynchronized flag, and the flag was set before the filling started. green is the default on linux now, so two tasks on two workers can reach a cold table together: the second sees the flag already set, returns, and reads a half-built table. for the sha tables that means a wrong digest or an index past the end, and concurrent pushes race the list's own length and buffer besides. all three are plain data, so they are module constants now, built while the module loads — before main's body runs and therefore before any task exists. that removes the flag, the init function, and the window, rather than guarding them: there is no first use to synchronize when the table is already complete. the sha tables also lose 255 lines of push calls in favour of the literal they always were. i could not make the race fire on this two-core box — wake affinity keeps channel-woken tasks on one worker, so they serialize — and the new case will pass before and after for that reason. it is here to pin the intent and to fail if a table ever goes back to being built lazily.
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
std/hash.pith,std/net/http2/hpack.pithandstd/net/http2/huffman.pitheach filled a shared lookup table on first use behind a
mutflag, and setthe flag before filling. with green the default on linux, two tasks on two
workers can reach a cold table together — the second sees the flag set,
returns, and reads a partially built table. for the sha round constants that
is a wrong digest or an out-of-bounds index; concurrent
pushalso races thelist's own length and buffer.
all three tables are plain data, so they are module constants now, built at
module load — before
main's body and therefore before any task exists. thatdeletes the flag, the init function and the window instead of locking them.
the sha tables shed 255 lines of
pushcalls for the literal they alwayswere.
what was tested
the crypto known-answer tests,
make teston both backends, both corpora,make run-examples,make docsite-check, andgrpc_chat+grpc_reflect,which exercise hpack and huffman per request. a new case has sixteen tasks
spin on a barrier and hit all three tables at the same instant, asserting a
single agreed answer; identical under green default, one worker, and
PITH_GREEN=0.notes
the race could not be made to fire on this two-core box: wake affinity keeps
channel-woken tasks on the same worker, so they serialize. the new case
therefore passes before and after — it pins the intent and fails if a table
returns to lazy construction.
test_h2_connection_reuseflaked once during aloaded battery run; it passes 6/6 under artificial load on this branch and
6/6 on main, so it is the known load flake rather than anything here.