adoolnuc / nmodbus

Automatically exported from code.google.com/p/nmodbus
0 stars 0 forks source link

Unhandeled Exception #30

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. just disconnecting other party

What version of the product are you using? On what operating system?
1.7 any os

Please provide any additional information below.

New suggestion to avoid unhandeled error (whitch is the whole idea of the 
method.

        internal void CatchExceptionAndRemoveMasterEndPoint(Action 
action, string endPoint)
        {
            if (action == null)
                throw new ArgumentNullException("action");
            if (endPoint == null)
                throw new ArgumentNullException
("endPoint");

            try
            {
                action.Invoke();
            }
            catch (IOException ioe)
            {
                _log.DebugFormat("IOException encountered 
in ReadHeaderCompleted - {0}", ioe.Message);
                ModbusMasterTcpConnectionClosed.IfNotNull
(e => e(EndPoint));

                SocketException socketException = 
ioe.InnerException as SocketException;
                if (socketException != null && 
socketException.ErrorCode == Modbus.ConnectionResetByPeer)
                {
                    _log.Debug("Socket Exception 
ConnectionResetByPeer, Master closed connection.");
                    return;
                }

                //throw;
            }
            catch (Exception e)
            {
                _log.Error("Unexpected exception 
encountered", e);
                //throw;
            }
        }

Original issue reported on code.google.com by fredrik....@isg.se on 29 May 2008 at 9:34

GoogleCodeExporter commented 9 years ago
The purpose of the method CatchExceptionAndRemoveMasterEndPoint is to catch
exceptions that we can expect to occur during normal operation and handle them
accordingly. For example, it is possible for a master slave TCP connection to be
closed during the middle of any message exchange (e.g. client is closed). When 
this
happens an IOException is thrown with a specific inner SocketException. We 
check for
this special case and handle it gracefully. 

Although catching all exceptions as you proposed will prevent the slave from
crashing, it will also mask legitimate bugs. 

In short, if the ModbusTcpSlave crashes it is a bug should be fixed.

Scott

Original comment by sja...@gmail.com on 29 May 2008 at 9:56

GoogleCodeExporter commented 9 years ago
You are of cause correct, but any call to CatchExceptionAndRemoveMasterEndPoint 
should be encapsulated in a try catch block in that case.

Regards,

       / F

Original comment by fredrik....@isg.se on 29 May 2008 at 10:07

GoogleCodeExporter commented 9 years ago
I'm not sure you're getting me. I'm already handling the exceptions I know 
about, if
any other unexpected exceptions occur I /want/ the slave to crash (then the bug
should be fixed). Therefore I do /not/ want to additionally wrap calls to
CatchExceptionAndRemoveMasterEndPoint in a try catch block.

Scott

Original comment by sja...@gmail.com on 29 May 2008 at 10:12

GoogleCodeExporter commented 9 years ago
I will try to send you a stack trace when the error occurs. It is super simple 
to 
get the error. Simply unplug the master from the network during 
communication....

      / Fredrik

Original comment by fredrik....@isg.se on 29 May 2008 at 10:22

GoogleCodeExporter commented 9 years ago
That would be great if you could send a stack trace. This is exactly the kind 
of bug
we don't want :)

Scott

Original comment by sja...@gmail.com on 29 May 2008 at 10:24

GoogleCodeExporter commented 9 years ago
No more response here...

Original comment by sja...@gmail.com on 24 Nov 2008 at 2:47

GoogleCodeExporter commented 9 years ago
Just did a new project with NModbus. If a master is disconnectet by means of 
for example pulling out a network cable, slave crashes.

See stack trace. As you can see it is not a call initiated by user code, and 
therefore the exception cannot be taken care of.

ERROR Got unexpected exception: Unable to read data from the transport 
connection: A connection attempt failed because the connected party did not 
properly respond after a period of time, or established connection failed 
because connected host has failed to respond.

StackTrace:    at System.Net.Sockets.NetworkStream.EndRead(IAsyncResult 
asyncResult) 
   at Modbus.Device.ModbusMasterTcpConnection.<>c__DisplayClass1.<ReadHeaderCompleted>b__0() 
   at Modbus.Device.ModbusMasterTcpConnection.CatchExceptionAndRemoveMasterEndPoint(Action action, String endPoint) 
   at Modbus.Device.ModbusMasterTcpConnection.ReadHeaderCompleted(IAsyncResult ar) 
   at System.Net.LazyAsyncResult.Complete(IntPtr userToken) 
   at System.Net.ContextAwareResult.CompleteCallback(Object state) 
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean ignoreSyncCtx) 
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state) 
   at System.Net.ContextAwareResult.Complete(IntPtr userToken) 
   at System.Net.LazyAsyncResult.ProtectedInvokeCallback(Object result, IntPtr userToken) 
   at System.Net.Sockets.BaseOverlappedAsyncResult.CompletionPortCallback(UInt32 errorCode, UInt32 numBytes, NativeOverlapped* nativeOverlapped) 
   at System.Threading._IOCompletionCallback.PerformIOCompletionCallback(UInt32 errorCode, UInt32 numBytes, NativeOverlapped* pOVERLAP)

Original comment by fredrik....@isg.se on 4 Nov 2011 at 10:18

GoogleCodeExporter commented 9 years ago
Reopening the issue since there is added information. 

Original comment by jke...@gmail.com on 10 Nov 2011 at 12:59

GoogleCodeExporter commented 9 years ago
I am having the same problem with CatchExceptionAndRemoveMasterEndPoint.
Assuming that I have understood the code correctly this method is executed in 
asynchronous callbacks only.
When you re-throw the exceptions this crashes the application.

I have a master device that uses modbus RTU over Ethernet i.e. it sends 
complete RTU packets using TCP as the transport; not Modbus/TCP packets.
I have successfully got it working by developing my own adapter for this.

I have discovered that when I use a standard Modbus/TCP slave (which my 
application must also support) the slave crashes my program when I send 
telegrams from this master.
The fault occurs in ReadFrameCompleted when 
ModbusMessageFactory.CreateModbusRequest throws a FormatException (which is 
expected as the packet format is incorrect).

This is captured by CatchExceptionAndRemoveMasterEndPoint but the master is not 
removed as this exception is not explicitly handled.
I think that because the method is executed in an asynchronous callback thread 
the re-throw of this exception causes the program to crash!

I am using NModus v1.11 (re-built for .net 4 cp but no other changes)
Application is built using VS2010 C# .

Any advice on how to best prevent this error would be welcome.

Nick

Original comment by nick.cr...@nov.com on 17 Jul 2012 at 12:59

GoogleCodeExporter commented 9 years ago
Has anyone found a fix or workaround for this problem?

Klaus

Original comment by k...@it-gmbh.de on 7 Aug 2012 at 5:21

GoogleCodeExporter commented 9 years ago
I tried to fix the problem by the following change to 
ModbusMasterTcpConnection.ReadFrameCompleted:

internal void ReadFrameCompleted(IAsyncResult ar)
{
    CatchExceptionAndRemoveMasterEndPoint(() =>
    {
// BEGIN CHANGE
        int bytesRead = Stream.EndRead(ar);
        _log.DebugFormat("Read Frame completed {0} bytes", bytesRead);
        if (bytesRead == 0)
        {
            _log.Debug("0 bytes read, Master has closed Socket connection.");
            EventHandler<TcpConnectionEventArgs> handler = ModbusMasterTcpConnectionClosed;
            if (handler != null)
                handler(this, new TcpConnectionEventArgs(EndPoint));
            return;
        }
// END CHANGE
        byte[] frame = CollectionUtility.Concat(_mbapHeader, _messageFrame);
        _log.InfoFormat("RX: {0}", StringUtility.Join(", ", frame));
// ...

I seems to work, but I'm not sure it is the correct solution

Klaus

Original comment by k...@it-gmbh.de on 10 Aug 2012 at 11:43

GoogleCodeExporter commented 9 years ago
Perhaps in the mind of the creator, the right solution may be :

        internal void CatchExceptionAndRemoveMasterEndPoint(Action action, string endPoint)
        {
// ......
        catch (IOException ioe)
            {
                _log.DebugFormat("IOException encountered in ReadHeaderCompleted - {0}", ioe.Message);
                ModbusMasterTcpConnectionClosed.Raise(this, new TcpConnectionEventArgs(EndPoint));

                SocketException socketException = ioe.InnerException as SocketException;
                if (socketException != null && socketException.ErrorCode == Modbus.ConnectionResetByPeer)
                {
                    _log.Debug("Socket Exception ConnectionResetByPeer, Master closed connection.");
                    return;
                }

//declare "public const int ConnectionAborted = 10053;" in Modbus.cs !!!
                if (socketException != null && socketException.ErrorCode == Modbus.ConnectionAborted)
                {
                    _log.Debug("Socket Exception ConnectionAborted, Master closed connection.");
                    return;
                }

                _log.Error("Unexpected IOException encountered", ioe);
                throw;
          }
//...........

// Take care to define:
// public const int ConnectionAborted = 10053;
// in Modbus.cs

Btw in my code i use a stronger approach handling ALL IOException disconnecting 
the master and going on silently.
Regards

nik

Original comment by nicola.p...@gmail.com on 5 Apr 2014 at 9:20

GoogleCodeExporter commented 9 years ago
Has anyone tested whether nik's solution above, works to fully address the 
issue? We're having this problem with our code as well and haven't been able to 
kill it. Or, nik, can you provide more information about the "stronger approach 
handling ALL IOException" you mentioned? 

Thank you,
Graeme

Original comment by graeme.h...@gmail.com on 17 Mar 2015 at 4:40