Frequency server supervisor

frequency.erl

%% Based on code from
%%   Erlang Programming
%%   Francecso Cesarini and Simon Thompson
%%   O'Reilly, 2008
%%   http://oreilly.com/catalog/9780596518189/
%%   http://www.erlangprogramming.org/
%%   (c) Francesco Cesarini and Simon Thompson

-module(frequency).
-export([start/0,allocate/0,deallocate/1,stop/0, client/1, supervisor_start/0]).
-export([init/1, client_loop/1, supervisor_init/0]).

%% These are the start functions used to create and
%% initialize the server.

start() ->
    spawn(frequency, init, [self()]).

%% The frequency server traps exits, so that it can link to client processes
%% and deallocate frequencies when they exit. This also allows clients to be
%% killed when the frequency server dies.
%% The catch with this is that the server doesn't die when its supervisor exits,
%% so as a workaround it takes the supervisor PID as an arg so that it can exit when
%% it gets an EXIT message from the supervisor.
%% Without this, I didn't see a way for both clients and the frequency server to be notified when
%% the other exits.
init(Supervisor) ->
  register(frequency, self()),
  process_flag(trap_exit, true),
  Frequencies = {get_frequencies(), []},
  loop(Supervisor, Frequencies).

% Hard Coded
get_frequencies() -> [10,11,12,13,14,15].

%% The Main Loop

loop(Supervisor, Frequencies) ->
  receive
    {request, Pid, allocate} ->
      {NewFrequencies, Reply} = allocate(Frequencies, Pid),
      Pid ! {reply, Reply},
      loop(Supervisor, NewFrequencies);
    {request, Pid , {deallocate, Freq}} ->
      NewFrequencies = deallocate(Frequencies, Freq),
      Pid ! {reply, ok},
      loop(Supervisor, NewFrequencies);
    {request, Pid, stop} ->
      Pid ! {reply, stopped};
    {'EXIT', Supervisor, _Reason} ->
      io:format("Supervisor died, going down!~n", []);
    {'EXIT', Pid, _Reason} ->
      io:format("trapped exit for pid ~w~n", [Pid]),
      NewFrequencies = exited(Frequencies, Pid),
      loop(Supervisor, NewFrequencies)
  end.

%% Functional interface

allocate() ->
    frequency ! {request, self(), allocate},
    receive
	    {reply, Reply} -> Reply
    end.

deallocate(Freq) ->
    frequency ! {request, self(), {deallocate, Freq}},
    receive
	    {reply, Reply} -> Reply
    end.

stop() ->
    frequency ! {request, self(), stop},
    receive
	    {reply, Reply} -> Reply
    end.


%% The Internal Help Functions used to allocate and
%% deallocate frequencies.

allocate({[], Allocated}, _Pid) ->
  {{[], Allocated}, {error, no_frequency}};
allocate({[Freq|Free], Allocated}, Pid) ->
  link(Pid),                                               %%% ADDED
  {{Free, [{Freq, Pid}|Allocated]}, {ok, Freq}}.

deallocate({Free, Allocated}, Freq) ->
  {value,{Freq,Pid}} = lists:keysearch(Freq,1,Allocated),  %%% ADDED
  unlink(Pid),                                             %%% ADDED
  NewAllocated=lists:keydelete(Freq, 1, Allocated),
  {[Freq|Free],  NewAllocated}.

exited({Free, Allocated}, Pid) ->                %%% FUNCTION ADDED
    io:format("exited ~w~n", [Pid]),
    case lists:keysearch(Pid,2,Allocated) of
      {value,{Freq,Pid}} ->
        NewAllocated = lists:keydelete(Freq,1,Allocated),
        {[Freq|Free],NewAllocated};
      false ->
        {Free,Allocated}
    end.

%% Starts the supervisor process.
supervisor_start() ->
    spawn(frequency, supervisor_init, []).

%% Starts the frequency server and start the supervisor loop.
supervisor_init() ->
    process_flag(trap_exit, true),
    Pid = spawn_link(frequency, init, [self()]),
    supervisor_loop(Pid).

%% Receives exit messages from the process it is supervising, and restarts it.
supervisor_loop(Pid) ->
    receive
        {'EXIT', Pid, _Reason} ->
            NewPid = spawn_link(frequency, init, [self()]),
            io:format("~w died, restarting as ~w~n", [Pid, NewPid]),
            supervisor_loop(NewPid)
    end.

%% Starts a simple frequency client.
client(Type) ->
    spawn(frequency, client_loop, [Type]).

%% Frequency client loop.
%% Allocates and deallocates frequencies at random intervals.
client_loop(Type) ->
    {ok, Freq} = allocate(),
    SleepTime = rand:uniform(500),
    timer:sleep(SleepTime),
    deallocate(Freq),
    client_loop(Type).

Hello,

I'm your reviewer. Your code looks almost identical to mine, which is nice because it makes things easier to review: I can actually follow what's going on!

Correctness

When I use the observer to kill the supervisor, the clients error out with:

Supervisor died, going down!

=ERROR REPORT==== 26-Apr-2017::22:29:11 ===
Error in process <0.37.0> with exit value: {badarg,[{frequency,deallocate,1,[{file,"frequency.erl"},{line,76}]},{frequency,client_loop,1,[{file,"frequency.erl"},{line,143}]}]}


=ERROR REPORT==== 26-Apr-2017::22:29:11 ===
Error in process <0.36.0> with exit value: {badarg,[{frequency,deallocate,1,[{file,"frequency.erl"},{line,76}]},{frequency,client_loop,1,[{file,"frequency.erl"},{line,143}]}]}

The assignment mentions:

we can’t expect the supervisor to restart the clients; on the other hand we can ensure that if the server terminates unexpectedly then the clients do too.

I interpreted that to mean that the clients should not be allowed to error out. However, I find the exercises and assignments pretty vague and confusing, so they are subject to multiple interpretations. My program differs from your code only in this respect:

loop(Supervisor, Frequencies) ->
  receive
    ...
    ...
    {'EXIT', Supervisor, _Reason} ->
      io:format("Supervisor died, going down!~n", []),
      exit(supervisor_failed);  %<=======*********** HERE *********

    {'EXIT', Pid, _Reason} ->
      io:format("trapped exit for pid ~w~n", [Pid]),
      NewFrequencies = exited(Frequencies, Pid),
      loop(Supervisor, NewFrequencies)
  end.

Adding that line will kill the server, which was going to terminate anyway, but it will also cause an exit signal to be sent to any linked clients, which will immediately kill the linked clients. However, even with that line in there, I think it's still possible for a client to error out: a client may not be linked to to the server when the server is killed (because the server just deallocated and unlinked the client), then the client could call allocate() before the server is restarted.

Like you, I didn't do anything special to preserve the "state" of the server. I had no idea what the discussion about the "state" of the server meant. Hmmm...I just thought of something. Maybe when the server is killed "the system" should preserve which frequencies were allocated at the time the server died, then on restart the server should not reallocate those frequencies? If a client were able to preserve which frequency it had when it died, then the client could reuse that frequency when it restarted itself? Who knows.

In client/1, the Type argument doesn’t seem to be used for anything, and I couldn't figure out what the name Type stood for. I sometimes have vestiges from previous attempts at a solution lurking in my code. Maybe that is what Type is?

Readability

Your code was very readable because it looked just like my code! I passed the Supervisor into loop() just like you did. I spent some time deciding between registering the supervisor and writing Supervisor = whereis(supervisor) at the top of loop() v. passing the Supervisor in as an argument. I decided that the repeated call to whereis() was probably less efficient.

A lot of the concurrent code posted in the comments by other students is too complex for me to follow. I can get a general idea of how their approach was different than mine, but that's about it. So it was nice to review some code that I could really understand!

Examining your system with Observer

• There’s no test function that starts a server and some clients, which would enable a reviewer to execute the test function, then start the observer and kill some processes to see how your program handles the crashes.

• There’s also no indication that a client is doing anything, for instance a print statement that shows that a client received a frequency and/or deallocated a frequency.

To test your code, all of that has to be added by a reviewer.

See you in class! I'm so far behind, I don't think many people are around.

Good luck with the rest of the class,
Seven