Skip to content

Commit

Permalink
Replace proc_lib:spawn_link/1 with spawn_link/1
Browse files Browse the repository at this point in the history 
Replace proc_lib:spawn_link/1 with just spawn_link/1 because the former
reports expected process exits as crashes if SASL is enabled. Reported
by Sonic Gao.
  • Loading branch information
@hdima
hdima committed Dec 14, 2013
1 parent 27fa08a commit 3008ee3
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGES
View file
@@ -1,5 +1,9 @@
Version 1.0.0beta (YYYY-MM-DD)

- Replaced proc_lib:spawn_link/1 with just spawn_link/1 because the former
reports expected process exits as crashes if SASL is enabled. Reported by
Sonic Gao.

- Fixed too restrictive checks for PYTHONPATH and RUBYLIB paths. Reported
by willemdj at GitHub.

Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS
View file
Expand Up @@ -8,3 +8,4 @@ Jacob Perkins <http://github.com/japerk>
Marko Mikulicic <http://github.com/mmikulicic>
Paul Bonser <http://github.com/pib>
willemdj at GitHub <https://github.com/willemdj>
Sonic Gao <https://github.com/sonicgao>
2 changes: 1 addition & 1 deletion src/erlport.erl
View file
Expand Up @@ -245,7 +245,7 @@ print(Data, State) ->
%% @doc Spawn a process to handle incoming call request
%%
spawn_call(Id, Module, Function, Args) ->
proc_lib:spawn_link(fun () ->
spawn_link(fun () ->
exit({Id, call_mfa(Module, Function, Args)})
end).

Expand Down

5 comments on commit 3008ee3

@maximvl
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hdima Why do you call exit explicitly instead of leaving process to finish after call_mfa/3?

@hdima
Copy link
Owner Author

@hdima hdima commented on 3008ee3 Dec 16, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maximvl Because I want to receive the return value. There's an example:

1> process_flag(trap_exit, true).
false
2> spawn_link(fun () -> result end).
<0.35.0>
3> flush().
Shell got {'EXIT',<0.35.0>,normal}
ok
4> spawn_link(fun () -> exit(result) end).
<0.38.0>
5> flush().
Shell got {'EXIT',<0.38.0>,result}
ok

@maximvl
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hdima But then sasl reports crash for every call from python, why not use monitors instead?

@hdima
Copy link
Owner Author

@hdima hdima commented on 3008ee3 Dec 16, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maximvl Don't really see your point here. This commit fixes the issue so now these processes won't be handled by SASL.

Originally I started with proc_lib:spawn_link/1 because it's a good idea to use it in most cases. However I missed the SASL case. In this commit proc_lib:spawn_link/1 was replaced with plain erlang:spawn_link/1 so now these processes won't be handled by SASL at all. There's also a way to fix it and still use proc_lib:spawn_link/1 but I decided that in this case erlang:spawn_link/1 will be better.

In this case it's better to use linked processes because it's a good idea to crash all the worker processes if main erlport process crashed. Also it doesn't matter for SASL if a process linked, monitored or neither - it's just reports crash if process exits with an unexpected exit value.

@maximvl
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hdima I see, problem is solved, thanks for explanation!

Please sign in to comment.