[erlang-questions] export record to mnesia module

[Mikael Pettersson]

On Sun, May 12, 2019 at 7:05 AM Peter J Etheridge <petergi@REDACTED> wrote:
>
> Dear Friends,
> Un & Pw are captured in a record Usr from a wx module.
> Usr is to be stored in mnesia in module(usrdb).
> export_fun() does not trouble Dialyzer. Mnesia starts, but does not create a new record;
>
> export_fun(Pw, Un, Pwv, Unv, Usr) ->
> case wxPasswordEntryDialog:connect(Pw, command_button_clicked, [{userData, Pwv}]) of
>   ok ->
>     case wxTextEntryDialog:connect(Un, command_button_clicked, [{userData, Unv}]) of
>       ok ->
>       register(usrdb, spawn_link(usrdb, handle_cast, [Usr])),
>         usrdb ! {send, Usr},
>         F = fun() ->
>         receive
>             Usr = #usr_record{un = Unv, pas = Pwv} ->
>                 mnesia:write(usr_table, Usr, write)
>                    after 0 ->
>         receive
>             Any ->
>                 Any
>                     end
>                 end
>             end,
>         mnesia:transaction(F)
>     end
> end,
> export_fun(Pw, Un, Pwv, Unv, Usr).
>
> What does this code lack?

This code has a number of problems.

1. The process executing export_fun/5 and F/0 expects to receive a
message, but you never pass its Pid anywhere, so it's unclear how
that's going to happen.  I'm assuming you intended to include self()
in the tuple sent to usrdb, and have usrdb send some #usr_record{}
back.

2. The F/0 function requires that a specifically-formatted message is
present in its calling process' mailbox, and if that's not the case,
e.g. due to timing between the processes involved, it throws away the
next available message.  This is very racy design.

3. The F/0 function's receive matches on variables already bound in
export_fun/5, so it will only match if sent a usr_record identical to
the one passed to export_fun/5, whose fields also exactly match the
other parameters passed to export_fun/5.  If this is intentional it's
highly redundant, if not you need to change the variable names to
avoid matching where you expect plain binding.

4. In each iteration you spawn and link a process, and register it as
userdb.  If that process terminates it will take the process executing
export_fun/5 down with it.  If it does not immediately terminate then
in the next iteration of export_fun/5 the attempt to register the next
instance of that process will crash the export_fun/5 process, and via
its link the previous usrdb process.  I cannot believe this behaviour
is intentional.

5. You spawn a process starting in usrdb:handle_cast/1.  A handle_cast
function would normally occur in a gen_server, and have arity 2.  And
you wouldn't start a gen_server that way.

6. You pass Usr twice to the usrdb process, both in the initial spawn
and then in an explicit message.  This looks odd and redundant.

7. Your attempt to issue a transaction executes the receive inside the
transaction function.  As receives aren't transactional operations, I
would have expected the transaction to be executed inside the
receive's successful path.

To start with, usrdb should probably be a semi-permanent separate
process, so you should't mess with spawn_link or register in the loop.
Also, the signalling between the processes is extremely fragile (and
incorrect as is), so you would benefit from changing usrdb to be a
standard gen_server and use gen_server:call or similar to communicate
with it.