Callisto82 / tftp.net

Implements the TFTP (Trivial File Transfer) protocol (client/server) in an easy-to-use C#/.NET library.
Microsoft Public License
80 stars 38 forks source link

Unhandled exception due to race condition in timer construction #30

Closed robeving closed 3 years ago

robeving commented 3 years ago

https://github.com/Callisto82/tftp.net/blob/bc70ff993471946dfbe4ceb0dbba499759136a8d/Tftp.Net/Transfer/TftpTransfer.cs#L32

You create the timer before the state variable is set. You should move the construction of the timer to the very last statement in the constructor. That way you can be sure that 'this' is valid.

In its current state it is possible to get an unhandled exception which crashes the program. You might think that this is rare but in my set up I can trigger this >30 times in 24 hours.

System.NullReferenceException: Object reference not set to an instance of an object.
   at Tftp.Net.Transfer.TftpTransfer.timer_OnTimer(Object context)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.TimerQueueTimer.CallCallback()
   at System.Threading.TimerQueueTimer.Fire()
   at System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()

As you are creating a timer you might want to consider an exception handler in the callback as any exception there will cause a unhandled exception and kill the program.