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

WebSeed: Pieces get out of order and create an http request with a negative end #638

Open
borigas opened this issue Feb 25, 2023 · 0 comments · May be fixed by #639
Open

WebSeed: Pieces get out of order and create an http request with a negative end #638

borigas opened this issue Feb 25, 2023 · 0 comments · May be fixed by #639

Comments

@borigas
Copy link
Contributor

borigas commented Feb 25, 2023

I don't completely understand what's happening, so I'm going to do my best to explain what I've seen and trying to step back from the symptom to find where things went wrong. I haven't spent much time in the core of the library, so I'm struggling to separate intended behavior from possible causes. Hopefully there's enough here for you to easily identify what I'm missing.

I'm seeing a peer sometimes failing to download when it's the only non-web seed peer. Downloading the same torrent repeatedly from the web seed sometimes repros but sometimes succeeds. I think the peer is getting into a state where certain blocks will continue to fail to download from the web seed.

Once it's in a failed state, putting a breakpoint at

msg.Headers.Range = new System.Net.Http.Headers.RangeHeaderValue (rr.startOffset, rr.startOffset + rr.count - 1);

shows rr.count < 0 because
WebRequests.Enqueue ((u, startOffset, endOffset - startOffset));
shows endOffset < startOffset. Requesting a negative range is immediately failing.

Stepping back to

RequestMessages.AddRange (bundle);
// The RequestMessages are always sequential
BlockInfo start = RequestMessages[0];
BlockInfo end = RequestMessages[RequestMessages.Count - 1];
CreateWebRequests (start, end);
and inspecting bundle, the BlockInfos are out of order. Sometimes this is a jump backwards (101,102,103,97,98) and sometimes this is sorted backwards (103,102,101,100). This seems to contradict the comment that they're always sequential, so seems like something might be wrong here.

Looking at StandardPicker, it seems to loop over the list of pieces in Requests.Values to choose PieceSegments. Requests.Values seems to be out of order. Looking at Requests.Values, it looks like it's stored as a Dictionary. I don't believe Dictionary is guaranteed to maintain order, so maybe that's part of what's going on? But it also looks like CanRequest returns pieces out of order based on need, so it seems most likely that StandardPicker is intended to return blocks in the best order, not necessarily sequential order.

Which leaves me wondering where the problem is. Is the best thing to do just sort RequestMessages? It looks like there's 2 places that are doing RequestMessages.RemoveAt (0);. If we reorder, maybe that would be a problem?

I also prototyped modifying RequestBundle.TryAppend() to only append if the block is a trailing block to prevent ever having a bundle go backwards. That seemed to fix my problem, but again I'm worried about unintended consequences of that change.

If there's more I can do to help diagnose, please let me know. Thanks for your work maintaining a great library. I really appreciate it.

@borigas borigas linked a pull request Mar 5, 2023 that will close this issue
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 a pull request may close this issue.

1 participant