Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finalizers which suspend are terminated immediately #212

Closed
halorgium opened this issue Apr 13, 2013 · 11 comments
Closed

Finalizers which suspend are terminated immediately #212

halorgium opened this issue Apr 13, 2013 · 11 comments
Labels
Milestone

Comments

@halorgium
Copy link
Contributor

When a finalizer runs and it's Task suspended, the finalizer is killed immediately after that time by the Actor#cleanup code.
In supervision groups, the finalizer is responsible for terminating other actors, which uses a SyncCall which suspends the task.
The main problem is that there is no more main loop to handle responses to method calls after the shutdown has started.

#<Thread:0x37e247e2 run>: nil: "start finalizer"
#<Celluloid::InternalPool::Thread:0x34c7da23 run>: "cd0813e7-6f56-4849-89ec-000000010000": "terminate #<Celluloid::TaskFiber:0x180c @type=:finalizer, @status=:callwait>"

This has implications and confusions on @jruby where Threads seem to stick around if you have ConditionVariables in use. See #203

@halorgium
Copy link
Contributor Author

Marking the Task as unsuspendable and raising if it is attempted.
But why is it in a Task to begin with?

@tarcieri
Copy link
Member

That's a good question... I was digging through the commit history trying to find why I made that change originally, and couldn't find it.

I think it'd make more sense to either run the finalizer in exclusive mode, or as you said, raise if anything attempts to suspend the current task.

@jjyr
Copy link

jjyr commented Apr 17, 2013

seems run finalizer in exclusive is better, user can just care the actor is alive or dead, not other.

@halorgium
Copy link
Contributor Author

@jjyr i would prefer to be able to detect the attempt to suspend.
It is possible to deadlock if we use exclusive.

What do you mean by?

user can just care the actor is alive or dead

@jjyr
Copy link

jjyr commented Apr 17, 2013

@halorgium i mean when i call the actor, i expected it be alive or dead, but not unsuspend
run finalizer in exclusive make i just call a dead actor, i think to introduce a new state(or just a new error) is not necessary
but i'm not sure about the deadlock

@halorgium
Copy link
Contributor Author

@jjyr the main problem I foresee is that at some point we will have to suspend the finalizer and allow new messages to arrive and be handled in the case of shutting down a tree of actors.

@jjyr
Copy link

jjyr commented Apr 17, 2013

@halorgium yes..i did not notice that...:neutral_face:

@halorgium
Copy link
Contributor Author

@jjyr do not worry, I am glad you have been providing feedback and looking at the code.
A lot of things in @celluloid are still uncertain and probably incorrect.
The best place to have some of these discussions might be in IRC. I hang out there quite a bit 😀

@tarcieri
Copy link
Member

A lot of things in @celluloid https://github.com/celluloid are still
uncertain and probably incorrect.

A lot of things in Akka and Erlang are still uncertain and probably
incorrect ;)

@halorgium
Copy link
Contributor Author

This is also true.

@halorgium
Copy link
Contributor Author

Task termination now logs, so users should make their finalizers exclusive if they need this behavior.

@digitalextremist digitalextremist modified the milestones: 1.0, 0.18.0 Jun 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants