use Task.Run to execute SmtpClient #74910
Conversation
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsI'm not sure why #74545 did not work. we got one more test hanging instance after it. This is only one test in contributes to #73447
|
| @@ -473,7 +473,7 @@ public async Task SendMailAsync_CanBeCanceled_CancellationToken() | |||
|
|
|||
| var message = new MailMessage("[email protected]", "[email protected]", "Foo", "Bar"); | |||
|
|
|||
| Task sendTask = client.SendMailAsync(message, cts.Token); | |||
| Task sendTask = Task.Run(() => client.SendMailAsync(message, cts.Token)); | |||
There was a problem hiding this comment.
I'm not clear on what these Task.Runs are meant to help. What's the theory about the problem and how this solves it?
If the issue is that the operation ends up completing before we get a chance to cancel it, we should change the loopback server to not have a race condition: rather than "some fake latency", make it configurable, and here make it infinite, such that no matter how long it takes for us to get to the cts.Cancel call, we know for sure that the operation won't have completed yet.
There was a problem hiding this comment.
The whole test hangs and I have no visibility why. When this happens Helix just kills the suite and we get no information. In #74545 I tried to add cap on how long we await for tasks - but that does not seems to be sufficient. So it feels we get stuck somewhere while creating the sendTask . So the goal is to put this to background so we can get to the await bellow and timeout ... and possibly collect some information. this is not attempt to fix the root cause.
There was a problem hiding this comment.
this is not attempt to fix the root cause.
Got it, ok, thanks.
src/libraries/System.Net.Mail/tests/Functional/SmtpClientTest.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Miha Zupan <[email protected]>
ghost
locked as resolved and limited conversation to collaborators
I'm not sure why #74545 did not work. we got one more test hanging instance after it.
After looking at the Mail code the
SendMailAsyncactually does fair amount of synchronous processing before returning theTask.I'm not sure if there is better way to do it @stephentoub but I'm pushing that to the thread-pool to be sure the code before it and final
awaitcan execute.This is only one test in
Mailtest suite using theManualResetEvent. I did look at and it seems ok to me. We basically block completion so the client has chance to cancel.contributes to #73447