1

My app is mutli thread real time handler of plc devices. It crashes sometimes with unhandled exception:

--- Unhandled Exception:
Type:ArgumentException
Message:Destination array is not long enough to copy all the items in the collection. Check array index and length.
Stack:   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.ObjectModel.Collection`1.System.Collections.ICollection.CopyTo(Array array, Int32 index)
   at System.Collections.ArrayList.InsertRange(Int32 index, ICollection c)
   at System.Collections.ArrayList.AddRange(ICollection c)
   at System.Collections.ArrayList..ctor(ICollection c)
   at System.Windows.Data.ListCollectionView.<RefreshOverride>b__1_0()
   at MS.Internal.Data.SynchronizationInfo.AccessCollection(IEnumerable collection, Action accessMethod, Boolean writeAccess)
   at System.Windows.Data.BindingOperations.AccessCollection(IEnumerable collection, Action accessMethod, Boolean writeAccess)
   at System.Windows.Data.ListCollectionView.RefreshOverride()
   at System.Windows.Data.CollectionView.RefreshInternal()
   at System.Windows.Data.CollectionView.RefreshOrDefer()
   at System.Windows.Data.ListCollectionView.ProcessCollectionChanged(NotifyCollectionChangedEventArgs args)
   at System.Windows.Data.CollectionView.ProcessChangeLog(ArrayList changeLog, Boolean processAll)
   at System.Windows.Data.CollectionView.ProcessInvoke(Object arg)
   at MS.Internal.Data.DataBindOperation.Invoke()
   at MS.Internal.Data.DataBindEngine.ProcessCrossThreadRequests()
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.DispatcherOperation.InvokeImpl()
   at System.Windows.Threading.DispatcherOperation.InvokeInSecurityContext(Object state)
   at MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(Object obj)
   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.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at MS.Internal.CulturePreservingExecutionContext.Run(CulturePreservingExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Windows.Threading.DispatcherOperation.Invoke()
   at System.Windows.Threading.Dispatcher.ProcessQueue()
   at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
   at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
   at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
   at System.Windows.Threading.Dispatcher.PushFrame(DispatcherFrame frame)
   at System.Windows.Application.RunDispatcher(Object ignore)
   at System.Windows.Application.RunInternal(Window window)
   at System.Windows.Application.Run(Window window)
   at System.Windows.Application.Run()
   at mes3cs.App.Main()

Only mes3cs.App.Main() is in my code.

I guess this is related to using ObservableCollection. I use it this way:

class DevMeasure {
    public readonly ObservableCollection<MsgMeasure> _measures = new ObservableCollection<MsgMeasure>();
    private readonly object _measuresLock = new object();
}

And in the init code:

        BindingOperations.EnableCollectionSynchronization(_measures, _measuresLock);

And show this _measures list in a DataGrid:

   dgMeasures.ItemsSource = _devMeasure._measures;

Why it is causing crash? I thought ObservableCollection is all thread safe, isn't it?

Rand Random
  • 7,300
  • 10
  • 40
  • 88
AIMIN PAN
  • 1,563
  • 1
  • 9
  • 13
  • Does this answer your question? [Destination array is not long enough to copy all the items in the collection. Check array index and length](https://stackoverflow.com/questions/43041839/destination-array-is-not-long-enough-to-copy-all-the-items-in-the-collection-ch) – Roe Jun 05 '23 at 09:48
  • `I thought ObservableCollection is all thread safe, isn't it?` why did you, though? first hit on google brings this up: https://stackoverflow.com/questions/23108045/how-to-make-observablecollection-thread-safe | Notice the edit of the top answer and why EnableCollectionSynchronization isn't enough – Rand Random Jun 05 '23 at 09:48

2 Answers2

4

I thought ObservableCollection is all thread safe, isn't it?

No! Unless otherwise documented, all types should be assumed to be non thread safe.

You have to ensure all access to the collection is synchronized. Simply calling EnableCollectionSynchronization is not enough, you must also lock the _measuresLock object whenever you are making any change to the collection. The EnableCollectionSynchronization call is simply there to tell wpf it also need to take a lock before accessing the collection. For detailed instructions, read the documentation.

However, I would consider just ensuring that all UI related code is running on the UI thread instead. Create some well defined, thread safe, interface for moving data between the worker threads and the UI thread, for example a blocking collection. That might help to create a clearer separation between background threads and the UI thread.

Note that multi threaded development is difficult. It introduces a bunch of new possible issues, and most of them are really difficult to debug. So you really need to know what you are doing to avoid problems in the first place. Making assumptions about classes being thread safe will very likely lead to bugs escaping to production where they could do real damage.

JonasH
  • 28,608
  • 2
  • 10
  • 23
0

Thanks all! I understand ObservableCollection is not thread safe now. Finally I wrote a MyObservableCollection to make it thread safe. And it has been running continuously for 1 week without crash.

using System.Collections.ObjectModel;
using System.Windows.Data;

namespace mes3cs.common
{
    public class MyObservableCollection<T> : ObservableCollection<T>
    {
        private object _lock = new object();
        public MyObservableCollection()
        {
            BindingOperations.EnableCollectionSynchronization(this, _lock);
        }
        new public void Add(T item)
        {
            lock (_lock)
            {
                base.Add(item);
            }
        }
        new public void Clear()
        {
            lock (_lock)
            {
                base.Clear();
            }
        }
        new public void CopyTo(T[] array, int arrayIndex)
        {
            lock (_lock)
            {
                base.CopyTo(array, arrayIndex);
            }
        }
        new public void Insert(int index, T item)
        {
            lock(_lock)
            {
                base.Insert(index, item);
            }
        }
        new public void InsertItem(int index, T item)
        {
            lock (_lock)
            {
                base.InsertItem(index, item);
            }
        }
        new public void Remove(T item)
        {
            lock (_lock)
            {
                base.Remove(item);
            }
        }
        new public void RemoveAt(int index)
        {
            lock (_lock)
            {
                base.RemoveAt(index);
            }
        }
        new public void RemoveItem(int index)
        {
            lock (_lock)
            {
                base.RemoveItem(index);
            }
        }
        new public void SetItem(int index, T item)
        {
            lock(_lock)
            {
                base.SetItem(index, item);
            }
        }
    }
}
AIMIN PAN
  • 1,563
  • 1
  • 9
  • 13