How to cancel (async)?
BornToBeRoot opened this issue · 8 comments
Is it possible to cancel an snmp query?
Maybe with a CancellationTokenSource?
Microsoft's current socket interface supports neither timeout nor cancellation for async operations, so what you asked is technically impossible.
You can refer to the timeout related thread for more information,
@lextm I know the socket doesn't support cancellation. You could take the CancellationToken and pass it down to where the Socket is created. Create a TaskCompletionSource, which the CancellationToken will cancel when it executes. Then use Task.WhenAny to await both the Socket operation and the TaskCompletionSource's Task. If the TaskCompletionSource Task completes then you can dispose the socket and trigger an exception.
Right now I can't use Async SNMP at all, since doing so will leak resources and my use case involves not entirely knowing if the remote end will respond.
I (very quickly) put together an example. It might be worth putting the TaskCompletionSource/Cancellation to where the Socket is actually used, but at the very least it allows for cancellation and should cleanup the socket resources.
Can you take a look at this code? Is it possible to implement?
I can try and put together a PR when I have some more free time. I personally needed the ability to send SNMP requests to large subjects where the majority of IPs wouldn’t respond... I really only needed SetAsync so I ended up using some reflection to access the needed non-public members (Like RequestCounter) and built my own static class. That way I could use this library without modification and get proper async, cancellation, and socket cleanup. I’ll try and post that code soon too.
@BornToBeRoot
Here was my workaround. https://gist.github.com/ncsurfus/b1b8dc14c1659e4acbc321a2e5b6ef48
@lextm are you considering adding a Cancellation Token to the Async methods?
The implementation suggested by @ncsurfus is really just registering a second Task (internally) that will "complete" upon cancellation - and then just wait for the first task to complete (the cancellation token task vs the snmp lib task) - effectively making us able to dispose the socket before we get a reply:
var tcs = new TaskCompletionSource<bool>();
using (var cancel = cancellationToken.Register(() => tcs.SetCanceled()))
using (var socket = receiver.GetSocket())
{
var response = request.GetResponseAsync(receiver, socket);
var result = await Task.WhenAny(tcs.Task, response);
if (result == tcs.Task)
throw new TaskCanceledException(response);
return await response;
}
I see a clear benefit to above approach (e.g. vs the "wrapping approach" described by @mr-giles in issue 23), as @ncsurfus approach will actually dispose the Socket and free up resources upon cancellation.
Above seems pretty straight forward and is working to my knowledge (a quick test). Is there a reason you don't add Cancellation Token like this? Are you worried about performance? or why is above suggestion not considered?
@NickNiebling I've been using this approach for some time now. I'm sending 256 - 512 SNMP GETs to different IP addresses at a time in a long running application. Most IPs will never respond. I haven't experienced any issues. Obviously, there is an additional heap allocation, but it's worth it in comparison to socket exhaustion. My workload has also been only on Windows.
Hopefully .NET Core 3.0 / .NET Standard 2.1 will include the appropriate CancellationToken methods for UDP operations.