sipsorcery icon indicating copy to clipboard operation
sipsorcery copied to clipboard

Never ever call "Receive" again,when an exception threw in m_udpSocket.BeginReceiveFrom,file of "SIPUDPChannel.cs"

Open wonderlive-c opened this issue 3 years ago • 3 comments

I found the comments is inaccurate,"the BeginReceiveMessageFrom will only throw if there is an problem with the arguments or the socket has been disposed of".When receive icmp destination port unreachable,it can cause an SocketException,this mean remote host does not listen on its port. But this is not local socket problem,we should not exit the Receive thread.

private void Receive()
        {
            try
            {
                EndPoint recvEndPoint = (ListeningIPAddress.AddressFamily == AddressFamily.InterNetwork) ? new IPEndPoint(IPAddress.Any, 0) : new IPEndPoint(IPAddress.IPv6Any, 0);
                m_udpSocket.BeginReceiveFrom(m_recvBuffer, 0, m_recvBuffer.Length, SocketFlags.None, ref recvEndPoint, EndReceiveFrom, null);
            }
            catch (ObjectDisposedException) { } // Thrown when socket is closed. Can be safely ignored.
            catch (Exception excp)
            {
                // From https://github.com/dotnet/corefx/blob/e99ec129cfd594d53f4390bf97d1d736cff6f860/src/System.Net.Sockets/src/System/Net/Sockets/Socket.cs#L3056
                // the BeginReceiveMessageFrom will only throw if there is an problem with the arguments or the socket has been disposed of. In that
                // case the socket can be considered to be unusable and there's no point trying another receive.
                logger.LogError($"Exception Receive. {excp.Message}");
                logger.LogDebug($"SIPUDPChannel socket on {ListeningEndPoint} listening halted.");
                Closed = true;
            }
        }

please fix it. Here is my code, I just call "EndReceiveFrom" again when catch a SocketException, is there a better way?


private void Receive()
        {
            try
            {
                EndPoint recvEndPoint = (ListeningIPAddress.AddressFamily == AddressFamily.InterNetwork) ? new IPEndPoint(IPAddress.Any, 0) : new IPEndPoint(IPAddress.IPv6Any, 0);
                m_udpSocket.BeginReceiveFrom(m_recvBuffer, 0, m_recvBuffer.Length, SocketFlags.None, ref recvEndPoint, EndReceiveFrom, null);
            }
            catch (ObjectDisposedException) { } // Thrown when socket is closed. Can be safely ignored.

            catch (SocketException socketException)
            {
                //ignored.
                logger.LogError(socketException, $"SocketException Receive. {socketException.Message}");
                EndReceiveFrom(null);
            }
            catch (Exception excp) when(excp is ArgumentException || excp is ArgumentNullException || excp is ArgumentOutOfRangeException || excp is InvalidOperationException)
            {
                // From https://github.com/dotnet/corefx/blob/e99ec129cfd594d53f4390bf97d1d736cff6f860/src/System.Net.Sockets/src/System/Net/Sockets/Socket.cs#L3056
                // the BeginReceiveMessageFrom will only throw if there is an problem with the arguments or the socket has been disposed of. In that
                // case the socket can be considered to be unusable and there's no point trying another receive.
                logger.LogError($"Exception Receive. {excp.Message}");
                logger.LogDebug($"SIPUDPChannel socket on {ListeningEndPoint} listening halted.");
                Closed = true;
            }
        }

wonderlive-c avatar Apr 13 '22 03:04 wonderlive-c

Probably related to #715. Seems the socket logic has been changed fairly substantially in .NET6 and there's some work to do this library to adjust to it.

sipsorcery avatar Apr 16 '22 15:04 sipsorcery

Probably related to #715. Seems the socket logic has been changed fairly substantially in .NET6 and there's some work to do this library to adjust to it.

I am using .Net5 framework,and I'v tried .NetCore3.1, Same situation.

wonderlive-c avatar Apr 19 '22 01:04 wonderlive-c

The socket logic has changed from when this class was written.

Previous: https://github.com/dotnet/corefx/blob/e99ec129cfd594d53f4390bf97d1d736cff6f860/src/System.Net.Sockets/src/System/Net/Sockets/Socket.cs#L3262

Now: https://github.com/dotnet/runtime/blob/e99fb185aa10ef177d19a51fd77b7a4b75db5395/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs#L2460

At the time it was written I spent a lot of time trying to work out the proper way to deal with ICMP packets with UDP sockets. One issue is #150 but there was more to it than that.

Coupled with this issue and #715 there is a chunk of work to do to update both the RTPChannel and SIPUDPChannel.

please fix it.

If only it was as easy as writing those words.

sipsorcery avatar Apr 23 '22 12:04 sipsorcery