Debugging gen_server, can you spot the error?

Chat / Discussions
1

I debugged a gen_server for several hours now. It uses a gen_tcp in server mode,
and when the client disconnected, the server lost all of its state.

Maybe you can spot the error quicker than I did :wink:

handle_info({tcp_closed, _Socket}, State = #state{mode=server}) ->
    [..]
    {noreply, #state{connected=false}};

Now I had a little heureka moment…

3 Likes
2 
State#state{ …

Instead of just

#state{
2 Likes
3

100 points to you!

2 Likes
4

This is where Dialyzer can help in interesting ways.

If you define your record as:

-record(state, {
  mode,
  connected
}).

Then anything you do is gonna be unchecked. If you do:

-record(state, {
  mode = server :: server | client
  connected = false :: boolean()
}).

Then it’s also not gonna do much except tell you when things are badly assigned (eg. connected=server) But if you instead avoid defaults:

-record(state, {
  mode :: server | client
  connected :: boolean()
}).

Then the call to #state{connected=false} should be invalid to Dialyzer because it implies #state{connected=false, mode=undefined}. By having no defaul.t values for elements that are to be dynamically configured and making sure you initialize things in the init/1 callback, times where you forget to pass in the original variable will detect sending in an uninitialized state.

You need a single field to make this work, and mandatory fields on maps should also be working the same (#{mandatory := type(), optional => type()}).

6 Likes
5

That’s incredibly helpful, thank you very much!
On top of the error checking, defining types for the record fields also servers a valuable documentation purpose.

I do use dialyzer from time to time on my codebase (it is some kind of a pet project at the moment, so there is no professional CI implemented), but I deserve another slap for plastering the dialyzer settings in rebar.config with 11 occurrences of no_<warning>…

1 Like
6

One minor drawback of leaving out defaults and specifying types is that in this case the record must be created in a single step, not incrementally - otherwise Dialyzer will complain. For huge records it’s sometimes annoying, so I ended up adding undefined for some types. Still, it’s better than nothing.

1 Like