vlingo-net / xoom-net-common

These are just a few common tools shared across various vlingo .NET projects.
Mozilla Public License 2.0
7 stars 9 forks source link

IScheduledAsync interface for TPL based implementation #23

Closed tjaskula closed 5 years ago

tjaskula commented 5 years ago

Add IScheduledAsync interface for dual implementation of async methods in vlingo-net-wire.

VaughnVernon commented 5 years ago

@tjaskula This should be unnecessary: IScheduledAsync. It exposes .NET threading/task facilities.

If you are trying to cause async scheduling/timer control on your Actor, don't forget to create an IScheduler of your Actor using SelfAs<IScheduled>(). This produces an actor proxy suitable sending asynchronous messages using the IScheduled protocol. The Scheduler will then send asynchronously IntervalSignal(...) messages to your Actor on timer intervals.

tjaskula commented 5 years ago

@VaughnVernon thank your for your feedback.

The point is that exposing Task or Task<> is completely acceptable and even advised in .NET libraries to signal an asynchronous method. It has nothing to do with threading, as Task doesn't mean thread, and you don't know if a new thread will be spun off or not. It's up to the underlying scheduler to decide. What the caller knows is that it has to await for some result in the feature. So many .NET librairies just exposes it, to enable asynchronous operations all the way down. Some examples:

What I'm trying to do, is not to create an sync wrapper on async operation which is very bad thing to do. Here is the example from vlingo-net-wire :

  1. I'm reading asynchronously from the Socket in SocketChannelSelectionProcessorActor with this line bytesRead = await channel.ReceiveAsync(new ArraySegment<byte>(readBuffer), SocketFlags.None); https://github.com/Luteceo/vlingo-net-wire/blob/43ce64a468e03db4dbfe68136df7c860a0d852b9/src/Vlingo.Wire/Channel/SocketChannelSelectionProcessorActor.cs#L177. This is using a built in asynchronous API returning Task, the way you do asynchronous programming in .NET right now. Because it uses the real async I/O under the hood. Otherwise you need to use synchronous blocking API.
  2. The recommendation is when your using on the bottom the async operation, all the chain (the whole callings methods) to the top should be asynchronous. Otherwise you need to block somewhere in the middle which is very bad leading to deadlocks and unexpected behavior.
  3. The point is that I reached the method exposed by IScheduler which is not asynchronous public void IntervalSignal(IScheduled scheduled, object data). Two options here, either I make its signature asynchronous and I propagate the asynchronous operation to the top, or I make a synchronous wrapper around asynchronous operation like I did : Task.Run(async () => await ProbeChannelAsync()); https://github.com/Luteceo/vlingo-net-wire/blob/43ce64a468e03db4dbfe68136df7c860a0d852b9/src/Vlingo.Wire/Channel/SocketChannelSelectionProcessorActor.cs#L101

So this is not ideal because I'm artificially offloading the work to threadpool when under the hood the Socket is using real IO asynchronous operation. But at least it won't deadlock.

Maybe I'm missing some concepts around IScheduled but I would be happy to discuss it how it can be improved, fixed without introducing this new interface

VaughnVernon commented 5 years ago

@tjaskula IScheduled is only used for scheduling timer events via Scheduler. It is not used for general purpose. Internally you should be dependent on actors for asynchronous behavior. It is understandable that lower level code such as with .NET network IO, you can use the idiomatic .NET and NETStandard libraries/APIs as needed, just as I did for the same in Java. Still, exposing Task, Task<>, etc. on the vlingo-net API is not in the spirit of the platform architecture.

tjaskula commented 5 years ago

It seems to me that in that case public void IntervalSignal(IScheduled scheduled, object data) is kind of top level method and will be called at some point by the scheduller in a fire-and-forget way.

Meaning I can wrap Task.Run(async () => await ProbeChannelAsync()).Wait(); call to offload to another thread as this won't block anyway because this is like I said the top level method.

Do you agree @VaughnVernon ?