1
Vote

Race Condition in Raise* calls

description

There are numerous race conditions in the Jamaa source code. Throughout the code, the following pattern is used to raise an event:

if (someEvent == null) return;
someEvent(this, new args());

This is inherently dangerous, because an event handler can disconnect from your event between the if statement and when the event is called. The thread-safe pattern is to create a copy of the event before invoking it:

EventHandler handler = someEvent;
if (handler != null)
handler(this, new args());


The specific bug we've run into is when we use the SmppClient in a using() block. We can dispose the object at the same time that the socket is closed, causing a race condition in TcpIpSession.RaiseSessionClosedEvent. This is an example of where the Jamaa library has an internal event handler that is being removed immediately before invocation.

There are other race conditions with disposing the SmppClient object around the use of System.Threading.Timer. SmppClient.vTimer will continue to fire after being disposed, and will throw an ObjectDisposedException in AutoReconnectTimerEventHandler:

System.ObjectDisposedException: Cannot access a disposed object.
at System.Threading.TimerQueueTimer.Change(UInt32 dueTime, UInt32 period)
at JamaaTech.Smpp.Net.Client.SmppClient.StartTimer()
at JamaaTech.Smpp.Net.Client.SmppClient.AutoReconnectTimerEventHandler(Object state)
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.TimerQueue.FireNextTimers()


The real threat of this in a timer callback is that this exception is not on an application controlled thread. This causes the .NET to forcibly close the entire application due to the uncaught exception.

comments