Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

raw option doesn't work gen_tcp when the socket backend is enabled (otp 24) #5122

Closed
dogtapdsd opened this issue Aug 16, 2021 · 5 comments · Fixed by #5151
Closed

raw option doesn't work gen_tcp when the socket backend is enabled (otp 24) #5122

dogtapdsd opened this issue Aug 16, 2021 · 5 comments · Fixed by #5151
Assignees
Labels
bug Issue is reported as a bug in progress team:PS Assigned to OTP team PS
Milestone

Comments

@dogtapdsd
Copy link

Describe the bug
https://bugs.erlang.org/browse/ERL-1287
this bug was fixed in otp 23.1, but happen again in otp 24, It cann't listening multiple sockets on the same port.

To Reproduce

Eshell V12.0.3 (abort with ^G)
1>
1> gen_tcp:listen(0, [{inet_backend, socket}, {raw, 1, 15, <<1:32/native>>}, {port, 8888}, {ip, any}]).
{ok,{'$inet',gen_tcp_socket,
{<0.82.0>,
{'$socket',#Ref<0.1220593668.1297743873.200397>}}}}
2>
2> gen_tcp:listen(0, [{inet_backend, socket}, {raw, 1, 15, <<1:32/native>>}, {port, 8888}, {ip, any}]).
{error,eaddrinuse}

Expected behavior
I test in otp 23.3 with no error

Eshell V11.2 (abort with ^G)
1> gen_tcp:listen(0, [{inet_backend, socket}, {raw, 1, 15, <<1:32/native>>}, {port, 8888}, {ip, any}]).
{ok,{'$inet',gen_tcp_socket,
{<0.83.0>,
{'$socket',#Ref<0.3300202661.3446013953.155967>}}}}
2>
2> gen_tcp:listen(0, [{inet_backend, socket}, {raw, 1, 15, <<1:32/native>>}, {port, 8888}, {ip, any}]).
{ok,{'$inet',gen_tcp_socket,
{<0.85.0>,
{'$socket',#Ref<0.3300202661.3446013953.155982>}}}}
3>

Affected versions
Erlang/OTP 24 [erts-12.0.3]

@dogtapdsd dogtapdsd added the bug Issue is reported as a bug label Aug 16, 2021
@rickard-green rickard-green added the team:PS Assigned to OTP team PS label Aug 17, 2021
@RaimoNiskanen RaimoNiskanen self-assigned this Aug 18, 2021
@RaimoNiskanen
Copy link
Contributor

RaimoNiskanen commented Aug 19, 2021

It seems that I refactored code when implementing fdopen/2 in gen_tcp_socket, and then setting level socket options was moved to after the bind(2) call. Now I will have to figure out if that was purely accidental or if there was some reason behind it.

That code refactoring causes your raw option (that by the way exists in non-raw form too {reuseaddr, true}), to be set after the address has been bound. Hence it has no effect. The same applies to the non-raw option too... So this is formally not the same bug that was fixed in ERL-1287.
Edit: your raw option is SO_REUSEPORT, not SO_REUSEADDR. My mistake.

I want to take a more thorough look at option handling in gen_tcp_socket, to see how to make it less messy...

@RaimoNiskanen
Copy link
Contributor

RaimoNiskanen commented Aug 19, 2021

This should be the smallest possible and probably correct fix:

diff --git a/lib/kernel/src/gen_tcp_socket.erl b/lib/kernel/src/gen_tcp_socket.erl
index 1841623241..5125bc4997 100644
--- a/lib/kernel/src/gen_tcp_socket.erl
+++ b/lib/kernel/src/gen_tcp_socket.erl
@@ -316,8 +316,8 @@ listen_open(Domain, ListenOpts, Opts, ExtraOpts, Backlog, BindAddr) ->
                   [{start_opts, StartOpts}] ++ SocketOpts ++ Setopts_1),
             ErrRef = make_ref(),
             try
-                ok(ErrRef, call_bind(Server, default_any(Domain, BindAddr))),
                 ok(ErrRef, call(Server, {setopts, Setopts})),
+                ok(ErrRef, call_bind(Server, default_any(Domain, BindAddr))),
                 Socket = val(ErrRef, call(Server, {listen, Backlog})),
                 {ok, ?MODULE_socket(Server, Socket)}
             catch

@dogtapdsd
Copy link
Author

dogtapdsd commented Aug 20, 2021

thanks for your reply,I will check those fix codes。

RaimoNiskanen added a commit that referenced this issue Sep 2, 2021
…OTP-17580' into maint

* raimo/kernel/inet-backend-socket-option-parsing/GH-5122/OTP-17580:
  Further simplify option parsing
  Comment out currently dead code due to Dialyzer
  Fix tests for inet_backend = socket
  Rewrite option parsing to less messy
@RaimoNiskanen RaimoNiskanen added this to the OTP-24.1 milestone Sep 2, 2021
@RaimoNiskanen
Copy link
Contributor

Fix merged

@hazardfn
Copy link

hazardfn commented Nov 8, 2023

I believe these symptoms have reappeared in OTP 26 (at least broken on 26.1.2). See: ninenines/ranch#216 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug in progress team:PS Assigned to OTP team PS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@rickard-green @RaimoNiskanen @hazardfn @dogtapdsd and others