dabeaz / curio

Good Curio!
Other
4.04k stars 243 forks source link

Getting rid of daemon tasks #174

Closed dabeaz closed 7 years ago

dabeaz commented 7 years ago

I'm thinking about getting rid of the concept of daemon tasks. Instead the run() method would simply run the supplied coroutine until it returns. At that point, the kernel would stop.

Not sure if this would break much stuff or not. However, not having daemon tasks would probably simplify curio. One less concept to worry about.

Thoughts?

njsmith commented 7 years ago

So compared to now, the changes would be (1) all tasks (except for the main task) effectively become daemon tasks, (2) daemon tasks are no longer cancelled at exit, but instead just killed unceremoniously?

dabeaz commented 7 years ago

The changes are that the run() method returns when the main task exits. Any remaining tasks would be kept around and resumed should therun() method be reinvoked. Otherwise, they'd simply be cancelled on kernel shutdown. Note: to my knowledge, there is no "unceremonious killing" in Curio. Tasks are always terminated via cancellation, even on kernel shutdown. This includes daemon tasks.

That said, I'm now not so sure on getting rid of daemon tasks. I looked into it yesterday and it caused all sorts of tests to fail. These can be fixed by properly waiting for all child tasks to finish (e.g., adding explicit task.join() calls). I guess I'd have to think about how important it is for a parent to properly wait for all of its child tasks to finish.

imrn commented 7 years ago

Let's have it outlined:

In the light of these

dabeaz commented 7 years ago

Currently, there's nothing special about daemon tasks except for the fact that the run() method doesn't wait around for them to finish.

imrn commented 7 years ago

Is it really an important reason against bulldozing them?

dabeaz commented 7 years ago

I don't know. I guess I'm just wondering if I could dispense with them altogether and tell people that if it's important for spawned tasks to finish, then you've got to wait for them. Otherwise, they'll just get cancelled.

Big picture: Not having daemon tasks is just one less thing to have to explain.

imrn commented 7 years ago

On 2/7/17, David Beazley notifications@github.com wrote:

I don't know. I guess I'm just wondering if I could dispense with them altogether and tell people that if it's important for spawned tasks to finish, then you've got to wait for them. Otherwise, they'll just get cancelled.

Huh? If the developer loves his children tasks too much then he should join() them or await the coro in the first place. That's Curio Basics, CB 101. Do we still consider them as arguments for the issue?

Big picture: Not having daemon tasks is just one less thing to have to explain.

Definitely.

jkbbwr commented 7 years ago

Realistically if you have task linking and you want daemonic tasks then just make your main task supervise them forever.

imrn commented 7 years ago

Realistically if you have task linking and you want daemonic tasks then just make your main task supervise them forever

No need to worry: Infact all tasks are now daemonic or daemons are now ordinary tasks. They will be around unless you perform explicit shutdown.

dabeaz commented 7 years ago

Huh? If the developer loves his child tasks to much then he should join() them or await the coro in the first place. That's Curio Basics, CB 101. Do we still consider them as arguments for the issue?

This might be the crux of it really. At the moment, you can be kind of sloppy in spawning tasks. You can spawn them and never wait around for them. Curio will still make sure they finish (provided they are non-daemonic). Maybe it's better to force people to think about joining with the tasks if completion is important.

dabeaz commented 7 years ago

For now, I'm keeping daemon tasks. In fooling around with it, I kept coming back to the "explicit is better than implicit" mantra from the Zen. Ultimately, I think it might be better (for understandability) to explicitly indicate that a task is intended to run the background (e.g. the daemon flag).

I might revisit the issue should Curio grow support for task groups, task linking, or some other concepts.

Closing for now.