Default value of `stop/3` is `:infinity`

[gausby]

This is the current implementation of stop/3

@spec stop(fsm_ref, reason :: term, timeout) :: :ok
def stop(fsm, reason \\ :normal, timeout \\ :infinity) do
  :gen.stop(fsm, reason, timeout)
end

I fear that the infinite timeout could cause processes to hang if something keep them around. Normally, as I recall, a timeout is set to 5 seconds.

I suggest that we change the default timeout to 5000 instead of :infinity.

[pavlos]

I thought that was strange as well, but I wanted to be consistent with GenServer.stop/3, which has a default timeout value of infinity (http://elixir-lang.org/docs/stable/elixir/GenServer.html#stop/3).

What do you think?

[gausby]

Yeah, GenServer has that behaviour. Personally I would go with the timeout of 5000, but it could be that the process will be terminated elsewhere if hanging too long? I got to ask around about this.

[pavlos]

Perhaps if the FSM is in a supervision tree (as it should be), the supervisor will clobber it?

[gausby]

@pavlos maybe, but I guess that the supervisor would try to call GenFSM.stop/3, which would give it a timeout, if my previous thinking about stop holds true. (theorising)

[pavlos]

Looks like in your child spec, you define shutdown settings, either a timeout value, or brutal_kill

http://erlang.org/doc/design_principles/sup_princ.html:

shutdown defines how a child process is to be terminated.

    brutal_kill means that the child process is unconditionally terminated using exit(Child, kill).
    An integer time-out value means that the supervisor tells the child process to terminate by calling exit(Child, shutdown) and then waits for an exit signal back. If no exit signal is received within the specified time, the child process is unconditionally terminated using exit(Child, kill).
    If the child process is another supervisor, it is to be set to infinity to give the subtree enough time to shut down. It is also allowed to set it to infinity, if the child process is a worker. See the warning below:

[pavlos]

Regardless of what the "right" answer is, should we stay consistent with Elixir GenServer, since GenFSM is quite similar? Or perhaps we should send Elixir a PR or issue to remove the infinity value and see what they say?

[gausby]

We can keep it like it is. I am fairly sure someone would have complained about the :infinity-default value in GenServer by now if it is a huge deal.

[gausby]

I'll close this because I think that staying consistant with the GenServer implementation is an excellent point.

[pavlos]

Ah yes, also, I just checked the erlang docs - default value of timeout is also infinity - http://erlang.org/doc/man/gen_fsm.html#stop-3

[gausby]

Hah. I've been staring me blind on the typespecs: I forgot to read what the small print says :)

Good to know.