zeromq / chumak

Pure Erlang implementation of ZeroMQ Message Transport Protocol.
Mozilla Public License 2.0
197 stars 47 forks source link

Misbehaves #24

Closed DavidAlphaFox closed 6 years ago

DavidAlphaFox commented 6 years ago

Dear author

I'm still working on the problem which can cause a server crashed when another side exits. I think here are main reasons:

  1. when a chumak_peer exited which is caused by tcp_close will cause chumak_socket to exit.
  2. when a gen_server:call is made to chumak_peer after it exited will cause the implementations to exit.

So I comment the code blew in chumak_socket

   %handle_info({'EXIT', PeerPid, {shutdown, _Reason}}, State) -> 
   %    exit_peer(PeerPid, State),
   %    {stop, normal, State};

Previous patch I pushed still contains bugs. In this push, I distinguish between error and empty and check the result of chumak_peer:incoming_queue_out in implementations.

So every implementations which use chumak_peer:incoming_queue_out will contain codes like below.

queue_ready(State, _Identity, PeerPid) ->
    case chumak_peer:incoming_queue_out(PeerPid) of
        {out, Multipart} ->
            {noreply,handle_queue_ready(State,Multipart)};
        empty ->
            {noreply,State};
        {error,Info}->
            error_logger:info_msg("can't get message out in ~p with reason: ~p~n",[chumak_sub,Info]),
            {noreply,State}
    end.

Thank you very much.

drozzy commented 6 years ago

Looks good. I will merge, but just one thing. Could you please remove this comment?

%% In server mode the socket should not exit when a peer shutdown except pair mode ?
%% Please consider it.
%handle_info({'EXIT', PeerPid, {shutdown, _Reason}}, State) ->
%    exit_peer(PeerPid, State),
%    {stop, normal, State};

If you feel deleting this code is ok - just do it. It is better for future developers if the source code has no todo-comments in it. If you feel like it is a potential future problem, please open an issue.

drozzy commented 6 years ago

Oh, and thanks for the patch!

DavidAlphaFox commented 6 years ago

Ok, I see. I will open an issue to discuss this problem on chumak_socket:handle_info, and my intended on peer_exit.