Aquire semaphore when network will be accessed #25
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.
Potential fix for #24 . The full fix required not just acquiring the semaphore in
cache.go
more guardedly, but also not locking the event inqueue.go
until after the network request is made.From a concurrency standpoint, the change to
queue.go
is safe because:queue.go
still only uses it under mutex, except for the read on e.requestAs a sanity check, I ran
go test -race -count=1 ./...
100 times against this patch and the race detector did not find issues. The bug reproduction repo (https://github.com/natenjoy/httprc-cache-issue) contains instructions in how to run this patch against a minimal server setup.I did see there is a
v2
branch, but I still opted to open this PR against themain
branch since latest tagged jwx package still uses v1.0.4. I was hoping if this change (eventually) looks mergeable that you could pull it over to thev2
branch.