1

The following code gives me an exception:

var snapshot = BluetoothCapture.Instance.Snapshot();
var allowedDevice = snapshot.FirstOrDefault( _ => some_expression );

Collection was modified; enumeration operation may not execute.

I thought I could use a lock to freeze the collection so that I can iterate through it. However, I'm still getting the same exception.

The class definition below has a Snapshot method that attempts this:

public partial class BluetoothCapture
{
    ...

    public void Capture()
    {
        _watcher = DeviceInformation.CreateWatcher();
        _watcher.Added += (s, e) => { _devices.Add(e); };
        _watcher.Start();
    }

    public IEnumerable<DeviceInformation> Snapshot()
    {
        lock (_devices)
        {
            return _devices.AsReadOnly();
        }
    }
}

Any suggestions?

Scott Nimrod
  • 11,206
  • 11
  • 54
  • 118
  • What's the type of `_devices`? – Camilo Terevinto Jun 06 '18 at 12:34
  • 1
    Did you even read about `lock`? It has *nothing* to do with that. – rory.ap Jun 06 '18 at 12:34
  • 1
    `_devices.Add(e);` at least this one should be also locked – Dmitry Pavliv Jun 06 '18 at 12:35
  • 1
    Note that `lock` is just a lock that *everyone* has to agree to use. It's like putting up a gate and saying "everybody that wants to access the property has to go through the gate, and we only allow one person on the property at any given time". In other words, there will be a queue at the gate waiting for whoever is on the property to get back out. But, the lock/gate doesn't stop anyone else just ignoring the gate altogether. All the code that you want to wait will have to use the lock as well, otherwise you end up with what you're now seeing. – Lasse V. Karlsen Jun 06 '18 at 12:55

2 Answers2

0

Lock is used when you need to stop a block of code to execute in multiple threads (stop the parallel execution). If Capture is called multiple times, so, yes, you can have a write being called before the previous one being finished.

You can use a ConcurrentBag. The ConcurrentBag is a list like object, but it is thread safe (none of generic list is). But, ConcurrentBag is unordered collection, so it does not guarantee ordering.

If you need some ordered list, you can see this link Thread safe collections in .NET

You can also make a "lock" in the Add (not in the get)

 _watcher.Added += (s, e) => { lock(_devices){_devices.Add(e); }};

But, if your app is running for a while, you can have a memory and performance problem (the add will be not async), even if the Capture is.

William Borgo
  • 394
  • 2
  • 11
0

lock is really very useful concept but only when we use it wisely.

If in further code, your don't want to update reference of snapshot (the collection which you are getting from BluetoothCapture.Instance.Snapshot()) but just perform some Linq query to get filtered value to perform some logic. you can avoid using lock.

It will be beneficial too, as by not doing lock you are actually not holding other threads to perform it's logic. - and we should not ignore the fact that terrible use of lock can cause serious problems like dead-lock too.

you are getting this exception, most likly, the collection on which you are performing linq query; is being updated by some other thread. (i too got that problem).

you can do one thing, instead of using general reference of collection (the one which you are getting from BluetoothCapture.Instance.Snapshot()) you can create a local list - as it is local it will not be updated by other threads.

Amit
  • 1,821
  • 1
  • 17
  • 30