iamkinetic / NEventSocket

A reactive FreeSwitch eventsocket library for .Net/.Net Framework (.net standard 2)
Mozilla Public License 2.0
26 stars 11 forks source link

SubscribeEvents may not be Thread-Safe #14

Open DanieleGhianda opened 2 years ago

DanieleGhianda commented 2 years ago

I was using InboundSocket to originate some calls and play Audio files, and sometimes i got an ThrowInvalidOperationException_ConcurrentOperationsNotSupported where I called Play(...).

Here is the call stack:

    System.ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported()
    System.Collections.Generic.HashSet<T>.FindItemIndex(T)
    System.Collections.Generic.HashSet<T>.Contains(T)
    NEventSocket.Sockets.EventSocket.SubscribeEvents.AnonymousMethod__28_0(NEventSocket.FreeSwitch.EventName)
    System.Linq.Enumerable.All<TSource>(System.Collections.Generic.IEnumerable<TSource>, System.Func<TSource, bool>)
    NEventSocket.Sockets.EventSocket.SubscribeEvents(NEventSocket.FreeSwitch.EventName[])
    NEventSocket.Sockets.EventSocket.ExecuteApplication(string, string, string, bool, bool, int)
    NEventSocket.ApplicationExtensions.Play(NEventSocket.Sockets.EventSocket, string, string, NEventSocket.FreeSwitch.PlayOptions)

I checked the library and I noticed the use of HashSet: private readonly HashSet<EventName> subscribedEvents = new HashSet<EventName>();

Am I missing something or is it really not Thread-Safe ?

danbarua commented 1 year ago

It should be a ConcurrentHashSet<T> or introduce a lock mechanism if it’s being used concurrently.

That said, you might be putting a lot of workload on one socket connection if you’re having concurrency issues.

If that socket connection fails, and it’s managing all of your telecoms traffic - it’s going to affect all of your traffic.

The library assumes a 1:1 socket to call relationship, but I do know some users are using one socket for everything.

You can also make use of freeswitch’s internal scheduler for handing off tasks in bulk.

Can you describe your use case?

danbarua commented 1 year ago

If this scenario is occurring in simple test code, then you may be missing an await somewhere, so the runtime will continue onto the next socket API call before the previous one has completed and reported back.