Beckhoff / TF6000_ADS_DOTNET_V5_Samples

Sample code for the Version 6.X series of the TwinCAT ADS .NET Packages
https://infosys.beckhoff.com/content/1033/tc3_ads.net/9407515403.html?id=6770980177009971601
BSD Zero Clause License
37 stars 15 forks source link

"Symbol already registered" exception when registering multiple notifications #49

Closed pitz-1m closed 1 year ago

pitz-1m commented 1 year ago

We've been seeing an occasional crash while registering a notification. The exception looks like this:

System.ArgumentException: Symbol already registered!
   at TwinCAT.Ads.TypeSystem.AdsNotificationCache.Add(ISymbol symbol, UInt32 handle, SymbolNotificationTypes notificationType, NotificationSettings settings)
   at TwinCAT.Ads.ValueAccess.AdsValueAccessor.RegisterNotification(ISymbol symbol, SymbolNotificationTypes type, NotificationSettings settings)
   at TwinCAT.Ads.ValueAccess.AdsValueAccessor.OnRegisterNotification(ISymbol symbol, SymbolNotificationTypes type, INotificationSettings settings)
   at TwinCAT.Ads.TypeSystem.Symbol.add_ValueChanged(EventHandler`1 value)
   at System.Reactive.Linq.ObservableImpl.ClassicEventProducer`2.AddHandler(TDelegate handler) in /_/Rx.NET/Source/src/System.Reactive/Linq/Observable/FromEvent.cs:line 360
   at System.Reactive.Linq.ObservableImpl.EventProducer`2.Session.AddHandler(TDelegate onNext) in /_/Rx.NET/Source/src/System.Reactive/Linq/Observable/FromEvent.cs:line 342

This seems to happen at a point where two notifications are being registered concurrently. Reading through decompiled library code... It looks like the notification handle (on which the collision occurs) is ultimately generated by the GetNextClientHandle method in TwinCAT.Ads.Internal.NotificationReceiver, which doesn't seem to be thread-safe and can generate duplicate handles. I could be way off here, only looked at it briefly.

pitz-1m commented 1 year ago

Looks like this is fixed in the latest release. Thanks.