Introduce typesafe actors abstraction

[qwwdfsad]

Fixes #87 and #169

[fvasco]

Is parameters as an output the final design?

Should the counter method return an Int?

#87 (comment)

[qwwdfsad]

Yes.
We discourage "send and wait" pattern as it's not an actor-ish way to do things, if you need it, you're probably using the wrong abstraction.
Actors come in a system, one actor is no actor. In the real world counter method would send its result to another actor (which then updates UI, prints it to the console etc.).

Nevertheless, for some integrations with existing code it's more or less trivial to write actAndResponse on top of act in 5-10 lines of code. But we don't want to provide it in the library to avoid misusages

[fvasco]

Thanks for the quick response,
do you consider a good idea providing an example of bad code?

Should it be better describe your point of view in documentation and provide a better example?

[fvasco]

CompletableDeferred<Int>().also { counter.counter(it) }.await()

This looks like a code pattern

[qwwdfsad]

It's a matter of taste.
For me this pattern is obscure

[elizarov]

Agree. We should actually rewrite actor example in the guide. Actors should never be written like that. You should never need to wait for a response to a particular actor's action.

[fvasco]

Same example here

https://github.com/Kotlin/kotlinx.coroutines/blob/master/coroutines-guide.md#actors

Following your reasoning the "counter" use case fits really better using a Mutex, otherwise you have to consider the feature of replying actor's messages.

[qwwdfsad]

do you consider a good idea providing an example of bad code?
Should it be better describe your point of view in documentation and provide a better example?

I think I will add a short section.
The problem with documentation and a lot of examples is that it's already too big to be fully readable, we're thinking how to rewrite it

[dave08]

Is there a way to confine the current Actor implementation to say, Android's Service lifecycle? And also how would notifying progress to the UI be handled? I've tried with the previous implementation, and had code riddled with nested CompletableDeferreds, just to keep the service alive (by blocking its thread) while the actors were finishing their job... I didn't end up using the design because of the many hard to track down bugs that introduced...

[qwwdfsad]

@dave08
Bind android lifecycle to a Job (with defining handlers onStop() = Job.cancel() etc.) and then use that job as a parent of all actors.
Could you please clarify the rest of your statement? The problem doesn't look clear: you have to use CompletableDeferred to "send and wait"? Application was terminated before actors finished their work?

[dave08]

@qwwdfsad I want Android to know when the actors are actually finished. If the actors are called on the main Service thread and don't block it, Android will think the Service finished its work. On the other hand, I need to fan out and fan in for different actor tasks. The main overseeing task should be reporting progress in a blocking way, until finished. So I had tried passing CompletableDeferreds to the actor chain, and await for them on the Services main thread, which blocks it and gives me progress... but it was a mess... Your point of using a Job is a different aspect, say if the Service must be interrupted even though it wasn't finished, then resources should be released etc... but I need that a regular run should complete normally.

[qwwdfsad]

@dave08

I want Android to know when the actors are actually finished. If the actors are called on the main Service thread and don't block it, Android will think the Service finished its work

runBlocking { actor.join() } from non-daemon thread is not applicable? Your architecture with overseeing task is not clear to me (I'm out of Android/your app context)

Could you extract your problem to a toy android app with a couple of classes? If so, I can rewrite it using actors (and probably will find a couple of bugs or API inconveniences) and add it as a sample project to the guide, some background job + UI progress bar + termination conditions are a perfect fit for actors and it would be great to have such example.

[LouisCAD]

@dave08 The Service thread in an Android app is the same thread as everywhere else, the main (UI) thread, that you should never block. If you're thinking about IntentServices, they have their own thread, but these are kind of deprecated since API 26.
You can't tell Android to wait when it's telling you to stop. You can just cancel work that should be cancelled in a best effort. If Android kills your process, everything is aborted anyway.

The best you can do in a Service is asking it to stop with stopSelf(), at which point onDestroy() will be invoked (but the application process may still stay alive)

[kevinherron]

Is this going to make it into the release with 1.3? Even with experimental annotations (are those transitive, it relies on channels...) that would be preferable.

[qwwdfsad]

@kevinherron

We are focused on "for 1.0 release" issues in order to make release 1.0 soon and not to break non-experimental things in the future.
Actors will come in the follow-up release.

[jcornaz]

Thank you very much for your work.

In the proposed design, I tend to consider error-prone the need to use act, since it can easily be forgotten without any help or warning from the compiler. And Forgetting to use act result in data-race, which is never easy to understand/spot.

class MyActor : Actor() {

   // I don't find very clear that doSomething may resume *before* the work being actually done. Since the function suspend, it would be natural (IMHO) to assume it resumes when the job is actually done.
   // It is also very important to not forget to use `act`. But it is easy to forget, compiler won't help and missing usages of `act` may be hard to spot.
   suspend fun doSomething() = act {
     // work...
   }
}

Personally I'd prefer something like this:

interface Actor<in T> {

  // Here it is explicit, that the method suspend only until the message has been accepted.
  suspend fun accept(message: T)
}

class MyActor : AbstractActor<MessageType>() {

  // Yes it is more verbose. But there's nothing special we can forget without getting compilation error.
  override suspend fun onMessage(message: MessageType) = when(message) {
	is DoSomething -> // work...
  }
}

[qwwdfsad]

@jcornaz
You can use TypedActor with method protected abstract suspend fun receive(message: T) (I will probably rename it when revisit this branch):

class MyActor : TypedActor<MessageType>() {
   override protected suspend fun receive(message: MessageType) = ...
}

Actor should be used when you need either multiple method parameters (without ugly pairs/sealed classes where they are not necessary) or multiple methods/handlers.

Your concern is understandable, I will update guide and documentation accordingly.
What about missing act, we will provide IDEA inspection for public methods on classes which extend Actor, but are not using act.

[jcornaz]

You can use TypedActor with method protected abstract suspend fun receive(message: T)

This is indeed somehow what I'm looking for. Except I still need to use act right?

Your concern is understandable, I will update guide and documentation accordingly. What about missing act, we will provide IDEA inspection for public methods on classes which extend Actor, but are not using act.

That sounds great, thanks.

[qwwdfsad]

Except I still need to use act right?

No, TypedActor doesn't have that method

[fvasco]

The missing feature for this use case is the native support for aspect programming, I consider act as an aspect applied on each public method.

Moreover act does not yet supports coroutine scopes, the new signature should be: protected suspend fun act(block: suspend CoroutineScope.() -> Unit).

[TimothyEarley]

Except I still need to use act right?

No, TypedActor doesn't have that method

Then I guess this example is wrong and act is not needed here.

[qwwdfsad]

yup, documentation leftover

[qwwdfsad]

Leaving this one for a reference, I will return to the actors as soon as Flow stabilize a bit.
But this API needs careful rethinking in the terms of structured concurrency (and missing extension constructors :))

[qwwdfsad]

https://www.reddit.com/r/Kotlin/comments/nm2eaf/kotlin_team_ama_3_ask_us_anything/gznacew?utm_source=share&utm_medium=web2x&context=3