Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dspinner: RecursiveKeys(): do not hang on cancellations #727

Merged
merged 2 commits into from
Nov 26, 2024

Conversation

hsanjuan
Copy link
Contributor

Per ipfs/kubo#10593, if no one is reading from the channel returned by RecursiveKeys() and the context is cancelled, streamIndex will hang indefinitely.

Proposed fix is to always select when attempting to write to the out channel. If the context is done and there is no one to read, we can abort.

@hsanjuan hsanjuan self-assigned this Nov 25, 2024
@hsanjuan hsanjuan requested a review from a team as a code owner November 25, 2024 20:25
Per ipfs/kubo#10593, if no one
is reading from the channel returned by RecursiveKeys() and the context is cancelled, streamIndex will hang indefinitely.

Proposed fix is to always select when attempting to write to the `out` channel. If the context is done and there is no one to read, we can abort.
Copy link

codecov bot commented Nov 25, 2024

Codecov Report

Attention: Patch coverage is 36.36364% with 7 lines in your changes missing coverage. Please review.

Project coverage is 60.30%. Comparing base (91c4d50) to head (7502f79).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pinning/pinner/dspinner/pin.go 36.36% 6 Missing and 1 partial ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #727      +/-   ##
==========================================
- Coverage   60.35%   60.30%   -0.06%     
==========================================
  Files         244      244              
  Lines       31029    31034       +5     
==========================================
- Hits        18728    18715      -13     
- Misses      10632    10650      +18     
  Partials     1669     1669              
Files with missing lines Coverage Δ
pinning/pinner/dspinner/pin.go 58.46% <36.36%> (-0.26%) ⬇️

... and 12 files with indirect coverage changes

@hsanjuan hsanjuan enabled auto-merge (squash) November 26, 2024 15:24
@hsanjuan hsanjuan merged commit 8673560 into main Nov 26, 2024
15 checks passed
@hsanjuan hsanjuan deleted the fix/kubo-10593 branch November 26, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants