Glimpse/Glimpse — pull request #676 by CGijbels (2013-12-13 10:59:37+00:00)
Title: Made ApplicationPersistenceStore thread-safe

As mentioned in #597 there are some issues when using the default `ApplicationPersistenceStore` from Glimpse in an environment where requests are building up quite rapidly (especially with a maximum queue size of 25 requests).

I added a loadtest disguised as a unittest that clearly illustrates the problem. If you change the lines with `lock (queueLock)` to `//lock (queueLock)` and run the test, then you'll get one of the following exceptions:

```
System.InvalidOperationException
Collection was modified after the enumerator was instantiated.

Server stack trace:
at System.Collections.Generic.Queue`1.Enumerator.MoveNext()
at System.Linq.Enumerable.FirstOrDefault(IEnumerable`1 source, Func`2 predicate)
at Glimpse.Core.Framework.ApplicationPersistenceStore.GetByRequestId(Guid requestId) in ApplicationPersistenceStore.cs
```

```
System.NullReferenceException
Object reference not set to an instance of an object.

Server stack trace:
at Glimpse.Core.Framework.ApplicationPersistenceStore.<>c__DisplayClass1.b__0(GlimpseRequest r) in ApplicationPersistenceStore.cs: line 77
at System.Linq.Enumerable.FirstOrDefault(IEnumerable`1 source, Func`2 predicate)
at Glimpse.Core.Framework.ApplicationPersistenceStore.GetByRequestId(Guid requestId) in ApplicationPersistenceStore.cs
```

```
System.ArgumentException
Source array was not long enough. Check srcIndex and length, and the array's lower bounds.

Server stack trace:
at System.Array.Copy(Array sourceArray, Int32 sourceIndex, Array destinationArray, Int32 destinationIndex, Int32 length, Boolean reliable)
at System.Collections.Generic.Queue`1.SetCapacity(Int32 capacity)
at System.Collections.Generic.Queue`1.Enqueue(T item)
at Glimpse.Core.Framework.ApplicationPersistenceStore.Save(GlimpseRequest request) in ApplicationPersistenceStore.cs
```

By adding locks when accessing `GlimpseRequests` the exceptions above won't occur anymore. The performance impact can be neglected and it is always better than throwing around exceptions, don't you think?

@nikmd23 Any reason why I should __not remove__ the passed in `IDataStore`? The queue is put inside the store with a private key and never retrieved from the store afterwards since it is kept as a private field inside the _singleton_ instance of the `ApplicationPersistenceStore` (is created only once by the `Factory` ...). Or do you see reasons why we might end up with multiple instances of the `ApplicationPersistenceStore` all sharing the same queue? Because if that would be the case, then we need to wrap that queue so that the locking is shared as well and put that wrapped queue inside the store, or we lock on the queue instance instead of on the private `queueLock` field, but then we need to adapt the code that puts it in the store in the first place, so that that code is thread safe as well. Plenty of options to choose from, but it would make it a lot clearer afterwards