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

Calling TcpListener.Create(int port) on multiple threads can fail #7087

Closed
Tragetaschen opened this issue Feb 16, 2018 · 4 comments
Assignees
Projects

Comments

@Tragetaschen
Copy link
Contributor

@Tragetaschen Tragetaschen commented Feb 16, 2018

Description

I have two ThreadPool threads (Task.Run(…)) that both call TcpListener.Create(int port) and then Start() with different ports each. Sometimes, these two calls fail with

System.Net.Sockets.SocketException (0x80004005): Protocol option not supported
  at System.Net.Sockets.Socket.GetSocketOption (System.Net.Sockets.SocketOptionLevel optionLevel, System.Net.Sockets.SocketOptionName optionName) [0x00020] in <a6842b19ae8449b7a2fd38fe764bcc1c>:0
  at System.Net.Sockets.Socket.get_DualMode () [0x0001a] in <a6842b19ae8449b7a2fd38fe764bcc1c>:0
  at System.Net.Sockets.Socket.get_IsDualMode () [0x0000a] in <a6842b19ae8449b7a2fd38fe764bcc1c>:0
  at System.Net.Sockets.Socket.RemapIPEndPoint (System.Net.IPEndPoint input) [0x00000] in <a6842b19ae8449b7a2fd38fe764bcc1c>:0
  at System.Net.Sockets.Socket.Bind (System.Net.EndPoint localEP) [0x0001e] in <a6842b19ae8449b7a2fd38fe764bcc1c>:0
  at System.Net.Sockets.TcpListener.Start (System.Int32 backlog) [0x00044] in <a6842b19ae8449b7a2fd38fe764bcc1c>:0
  at System.Net.Sockets.TcpListener.Start () [0x00000] in <a6842b19ae8449b7a2fd38fe764bcc1c>:0

and

System.Net.Sockets.SocketException (0x80004005): Protocol option not supported
  at System.Net.Sockets.Socket.SetSocketOption (System.Net.Sockets.SocketOptionLevel optionLevel, System.Net.Sockets.SocketOptionName optionName, System.Int32 optionValue) [0x0002f] in <a6842b19ae8449b7a2fd38fe764bcc1c>:0
  at System.Net.Sockets.Socket.set_DualMode (System.Boolean value) [0x0001a] in <a6842b19ae8449b7a2fd38fe764bcc1c>:0
  at System.Net.Sockets.TcpListener.Create (System.Int32 port) [0x0002a] in <a6842b19ae8449b7a2fd38fe764bcc1c>:0

respectively.

If that happened, every call to TcpListener.Create(int port) will throw the second exception.

I compile mono from source for my ARM platform and so was able to put some debug code in mono/metadata/w32socket.c when ves_icall_System_Net_Sockets_Socket_SetSocketOption_internal sets the error close to the and. At that point the relevant variables are soc==41, level==27 (expected values for SocketOptionLevel.IPv6 and SocketOptionName.IPv6Only), while the two system values are system_level==0 and system_name==26. The latter value is the expected IPV6_ONLY value, while the former is SOL_IP which is clearly wrong - hence the failure.

I then checked fetch_protocol in mono/utils/networking-posix.c: During application startup, I can see it being called thrice without a cache value: once for "ip" and twice for "ipv6". The first call for ipv6 returns the expected 41, the second call sets the cached proto value to 0 which is then retained for the rest of the program. So it looks like a reentrency problem of getprotobyname (which isn't reentrant).

I don't know how to fix this for the full breadth of supported platforms, and instead replaced every call to fetch_protocol with the expected constant value (6, 0, and 41). This fixes the issue on my platform.

On which platforms did you notice this

[ ] macOS
[x ] Linux
[ ] Windows

Version Used:

Mono JIT compiler version 5.4.1.6 (tarball Fri Feb 16 18:52:03 CET 2018)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
        TLS:           __thread
        SIGSEGV:       normal
        Notifications: epoll
        Architecture:  armel,vfp+hard
        Disabled:      none
        Misc:          softdebug
        LLVM:          supported, not enabled.
        GC:            sgen (concurrent by default)
@marek-safar marek-safar added this to Bugs Pool in Bugs Week via automation Feb 20, 2018
@marek-safar

This comment has been minimized.

Copy link
Member

@marek-safar marek-safar commented Feb 20, 2018

Yeah, the different probing methods should use 3 distinguish cache and result values.

@alexischr

This comment has been minimized.

Copy link
Member

@alexischr alexischr commented Aug 16, 2018

@Tragetaschen I could not reproduce the issue (I used something like https://gist.github.com/alexischr/f57083a870c54b95afd594e85d922616), but the above PR should prevent getprotobyname() from being re-entered. Could you try out the patch in your environment if you have the chance? That file is pretty stable so this should apply: https://patch-diff.githubusercontent.com/raw/mono/mono/pull/10158.patch

@alexischr

This comment has been minimized.

Copy link
Member

@alexischr alexischr commented Aug 17, 2018

@Tragetaschen @jaykrell found getprotobyname_r() which makes things much nicer, please try https://patch-diff.githubusercontent.com/raw/mono/mono/pull/10163.patch instead

alexischr added a commit to alexischr/mono that referenced this issue Aug 17, 2018
alexischr added a commit to alexischr/mono that referenced this issue Aug 17, 2018
alexischr added a commit to alexischr/mono that referenced this issue Aug 17, 2018
alexischr added a commit to alexischr/mono that referenced this issue Aug 17, 2018
Bugs Week automation moved this from In Progress to Done Aug 22, 2018
alexischr added a commit that referenced this issue Aug 22, 2018
@Tragetaschen

This comment has been minimized.

Copy link
Contributor Author

@Tragetaschen Tragetaschen commented Aug 22, 2018

Nice!

@marek-safar marek-safar moved this from Done to Archived in Bugs Week Sep 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Bugs Week
Archived
4 participants
You can’t perform that action at this time.