Why is this code broken?
source link: https://ayende.com/blog/196930-B/challenge-why-is-this-code-broken?Key=4a38b1d1-58e2-47da-9a27-3b73cae0ec39
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Comments
12:25 PM
Maybe using the same HttpClient in parallels request can overuse the socket? The HttpClient is not disposed? Maybe related with bad handling tasks? OpenRead stream is not seeked and is reading the file from the end, causing sending empty file?
I don't thing that will be related with the issue, but why declaring var file = f instead of doing directly in the foreach foreach (var file in files)?
12:41 PM
Ivan,
That flle = f there is here to avoid variable capture issue, see: https://stackoverflow.com/questions/271440/captured-variable-in-a-loop-in-c-sharp
13:05 PM
@Oren Eini - on the file = f - you haven't had to do that in a foreach since c# 5, where they made a breaking change in order to have the captured loop variable work "as expected".
Your code's primary issue is using Task.Factory.StartNew with an async delegate. In this case you're waiting on a list of Task<Task>s, so you're waiting on the wrong thing. I suppose you could unwrap them - but I wouldn't recommend using StartNew at all. Especially since you're now at its mercy for a task scheduler.
13:07 PM
Philip R,
You are correct, and I'm shocked. I wasn't aware of this change.
13:14 PM
It doesn't check the status of the response. PostAsync won't throw an exception on a non-successful HTTP status code. A call to EnsureSuccessStatusCode() or similar mechanism should be added. Also why does it use Task.Factory.StartNew? Wouldn't just calling an async method be better?
13:28 PM
Task.Factory.StartNew doesn't have an overload that takes a Func<Task>, so the async () => ... lambda is actually an async void method. It won't actually wait until the transfer is complete.
13:35 PM
Those are issues, but actually relatively minor ones. Thomas got it.
13:35 PM
Thomas,
Exactly right!
09:00 AM
My initial impression was that the code is broken by the use of async/await. Happy to see that it was the case :)
Join the conversation...
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK