KalebDark / lidgren-network-gen3

Automatically exported from code.google.com/p/lidgren-network-gen3
0 stars 0 forks source link

Exception thrown from thread #26

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Look at this line 
http://code.google.com/p/lidgren-network-gen3/source/browse/trunk/Lidgren.Networ
k/NetPeer.cs#144 :

m_networkThread = new Thread(new ThreadStart(Run));

And then closely to Run method:
http://code.google.com/p/lidgren-network-gen3/source/browse/trunk/Lidgren.Networ
k/NetPeer.Internal.cs#73

In this method when you cannot bind socket or on any other problem exception is 
thrown outside the thread !!

Here: 
http://code.google.com/p/lidgren-network-gen3/source/browse/trunk/Lidgren.Networ
k/NetPeer.Internal.cs#149

Now, what happens, noone can handle this exception and program aborts!

It is better to write a log instead.
Status property can give the information whether Run is called successfully.

Proposed solution:

catch (SocketException ex)
{
 if (sex.SocketErrorCode == SocketError.AddresssAlreadyInUse)
   LogError(...)
 else
  LogError(sex.Message);
 return;
}
catch (Exception ex)
{
 LogError(...)
 return;
}

Original issue reported on code.google.com by NN1436401@gmail.com on 10 Aug 2010 at 10:28

GoogleCodeExporter commented 9 years ago
Good thinking! Revision 94 now logs error in release instead of throwing 
exception

Original comment by lidg...@gmail.com on 10 Aug 2010 at 11:22

GoogleCodeExporter commented 9 years ago
http://code.google.com/p/lidgren-network-gen3/source/browse/trunk/Lidgren.Networ
k/NetPeer.Internal.cs?spec=svn94&r=94#142

throw new Exception("borak!");

I think it was not supposed to be.

#if DEBUG
                                catch(Exception ex)
                                {
                                        throw;
                                }
#else

First you can just use catch { throw; }
Second, you must not throw exception from thread even in Debug because it 
aborts the program :(

Original comment by NN1436401@gmail.com on 10 Aug 2010 at 11:35

GoogleCodeExporter commented 9 years ago
Yes; I accidently left some testing code in; patched it 30 seconds later tho in 
r95. In debug you want an exception to be thrown to be able to debug the 
problem; the debugger will break and you can inspect various threads.

Original comment by lidg...@gmail.com on 10 Aug 2010 at 11:47

GoogleCodeExporter commented 9 years ago
I disagree with you about debug mode.

I run program with debugger and when it reaches throw; I get "SocketException 
was unhandled" and I cannot continue running my program.

If you want to point a user out in the debug about error, you should use 
System.Diagnostics.Debug.Fail.

Original comment by NN1436401@gmail.com on 10 Aug 2010 at 12:00

GoogleCodeExporter commented 9 years ago
What's the point of continuing running the program? Since initialization failed 
unexpectedly we cannot keep the network thread alive and for a networked app I 
don't see the point in continuing.
Swallowing the exception in RELEASE is bad enough, but since there's no way of 
catching it easily it's the only way. In DEBUG however you'd want to break as 
early as possible.

The only almost-valid case I see is probing to see if the port is already in 
use; I should probably create a utility method for that.

Original comment by lidg...@gmail.com on 10 Aug 2010 at 12:30

GoogleCodeExporter commented 9 years ago
What about case of trying some different end points ? I mean if you try the 
first one and if it fails moves to another one.
Throwing exception there just aborts the program and doesn't allow me to handle 
it in any way.

Is it possible to move binding socket outside the thread ?
Than you just throw exception in the same thread where Start is called.
Everyone is happy.

Original comment by NN1436401@gmail.com on 10 Aug 2010 at 12:52

GoogleCodeExporter commented 9 years ago
Hmm, yes binding on main thread could be a good idea. I'll do some testing and 
make it so unless there's other issues. 

Original comment by lidg...@gmail.com on 10 Aug 2010 at 1:12