[Proposal] Warning when `GenServer.stop` is called with the current processes' pid

[Josh B]

Just ran into a nasty little bug that I thought I would share to see if there was some reasonable warning that could be made.

Here is an example IEx session:

```
Erlang/OTP 20 [erts-9.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:10] [hipe] [kernel-poll:false] [dtrace]

Interactive Elixir (1.6.0-dev) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> defmodule MyServer do
...(1)>   use GenServer
...(1)>
...(1)>   def handle_call(:quit, from, _) do
...(1)>     GenServer.reply(from, :ok)
...(1)>     GenServer.stop(self(), :normal)
...(1)>   end
...(1)>
...(1)>   def handle_call(:hello, _from, _) do
...(1)>     IO.inspect :world
...(1)>     {:reply, :ok, []}
...(1)>   end
...(1)> end
{
  :module,
  MyServer,
  <<70, 79, 82, 49, 0, 0, 14, 0, 66, 69, 65, 77, 65, 116, 85, 56, 0, 0, 1, 189,
    0, 0, 0, 46, 15, 69, 108, 105, 120, 105, 114, 46, 77, 121, 83, 101, 114,
    118, 101, 114, 8, 95, 95, 105, 110, 102, 111, ...>>,
  {:handle_call, 3}
}
iex(2)> {:ok, pid} = GenServer.start_link(MyServer, [])
{:ok, #PID<0.103.0>}
iex(3)> GenServer.call(pid, :hello)
:world
:ok
iex(4)> GenServer.call(pid, :quit)
:ok
iex(5)> GenServer.call(pid, :hello)
** (exit) exited in: GenServer.call(#PID<0.103.0>, :hello, 5000)
    ** (EXIT) time out
    (elixir) lib/gen_server.ex:790: GenServer.call/3
iex(5)> Process.alive?(pid)
true
```

Here I made a simple GenServer that responds to `:hello` and `:quit`. The code may look innocuous at first glance, but the GenServer deadlocks on the `:quit` call because `GenServer.stop` is synchronous and blocking by default. The correct way to implement this is by returning a stop tuple of course, however I was surprised that no warning or error was raised.

By contrast, a GenServer will crash if it tries to call itself:

```15:23:25.018 [error] GenServer #PID<0.97.0> terminating
** (stop) exited in: GenServer.call(#PID<0.97.0>, :call_self, 5000)
    ** (EXIT) process attempted to call itself
    (elixir) lib/gen_server.ex:783: GenServer.call/3
    (stdlib) gen_server.erl:636: :gen_server.try_handle_call/4
    (stdlib) gen_server.erl:665: :gen_server.handle_msg/6
    (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3
Last message (from #PID<0.84.0>): :call_self
State: []
Client #PID<0.84.0> is alive
    (stdlib) gen.erl:169: :gen.do_call/4
    (elixir) lib/gen_server.ex:787: GenServer.call/3
    (stdlib) erl_eval.erl:670: :erl_eval.do_apply/6
    (elixir) src/elixir.erl:233: :elixir.eval_forms/4
    (iex) lib/iex/evaluator.ex:245: IEx.Evaluator.handle_eval/5
    (iex) lib/iex/evaluator.ex:225: IEx.Evaluator.do_eval/3
    (iex) lib/iex/evaluator.ex:203: IEx.Evaluator.eval/3
    (iex) lib/iex/evaluator.ex:89: IEx.Evaluator.loop/1
```

`GenServer.stop` involves a middleman process from what I can see which stops this deadlock prevention from saving us in our original case.

Anyways, it would be nice to have Elixir (or Erlang) warn us when we are doing this. I could see this being implemented a few different places but wanted to put the problem out there to see what people thought

[Allen Madsen]

This seems like a good idea to me. A very simple solution I imagine is, here:

https://github.com/elixir-lang/elixir/blob/master/lib/elixir/lib/gen_server.ex#L751

Add a check if server == self() and raise. Since this delegates directly to :gen, it would be worthwhile to propose something similar for Erlang.

If there's some caveat to why this needs to be allowed, I think at least documentation for GenServer.stop/3 would be worthwhile.

Allen Madsen
http://www.allenmadsen.com

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-core+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/09dec8bd-393c-41ba-af12-65cca5f71bf3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[José Valim]

Yup, please open up an issue!

--

José Valim

www.plataformatec.com.br

Founder and 

Director of R&D

[Robert Virding]

I would be a bit formal and say that the reason is of course that GenServer:stop is a function that is meant to be called from a client and a callback which wants the stop the server returns a :stop tuple. Seriously, I don't see the problem. I see that GenServer.call does this but here I still don't see the problem. You will never be able to protect the user from themself.

Robert

[José Valim]

The fact we will never be able to protect the user from themselves does not mean we should not provide feedback or that we should not help them understand the problem when we can. For an experienced developer those mistakes can be somewhat obvious but for someone starting it out they can get quite puzzling.

José Valim

www.plataformatec.com.br

Founder and 

Director of R&D

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-core+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/12f8e14a-4733-4e05-bb74-2b9b1aee3add%40googlegroups.com.

[Mike Martinson]

I remember this being a gotcha for me when I was getting started. A nudge in the right direction would definitely have been helpful.

On Tuesday, 31 October 2017 03:58:37 UTC-7, José Valim wrote:

The fact we will never be able to protect the user from themselves does not mean we should not provide feedback or that we should not help them understand the problem when we can. For an experienced developer those mistakes can be somewhat obvious but for someone starting it out they can get quite puzzling.

José Valim

www.plataformatec.com.br

Founder and 

Director of R&D

On Tue, Oct 31, 2017 at 11:55 AM, Robert Virding <rvir...@gmail.com> wrote:

I would be a bit formal and say that the reason is of course that GenServer:stop is a function that is meant to be called from a client and a callback which wants the stop the server returns a :stop tuple. Seriously, I don't see the problem. I see that GenServer.call does this but here I still don't see the problem. You will never be able to protect the user from themself.

Robert

On Thursday, 26 October 2017 01:35:03 UTC+2, José Valim wrote:

Yup, please open up an issue!

--

José Valim

www.plataformatec.com.br

Founder and 

Director of R&D

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.

To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.