Fix: Collection was modified exception

[theramis]

Problem

We are using wiremock as part of our tests and we found that if we have tests running in parallel we sometimes get an error when calling the FindLogEntries method.

Error

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.

Potential Solution

This is my quick attempt at fixing the above issue. By convert the collection into a new list it shouldn't throw an error if the LogEntries collection is updated.
From a quick google around the ToList method itself isn't thread safe either though. So its possible that while the ToList method is creating a copy, it fails due to the original collection getting changed.

Thoughts?
@StefH

[codecov]

Codecov Report

Merging #331 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master     #331   +/-   ##
=======================================
  Coverage   80.08%   80.08%           
=======================================
  Files         111      111           
  Lines        4423     4423           
=======================================
  Hits         3542     3542           
  Misses        881      881

Impacted Files	Coverage Δ
...WireMock.Net/Server/FluentMockServer.LogEntries.cs	16.66% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19ee3c6...f80debc. Read the comment docs.

[StefH]

Can you take a look ?

[StefH]

I think it's even better to just use the property LogEntries here (that property is already made safe...)

Also in the DeleteLogEntry() method, it's safer to use the LogEntries property.

[theramis]

I think it's even better to just use the property LogEntries here (that property is already made safe...)

Adding/Removing/Setting/Clearing is thread safe for the LogEntries property (due to https://github.com/WireMock-Net/WireMock.Net/blob/master/src/WireMock.Net/Util/ConcurrentObservableCollection.cs) but enumerating is not.

In my case we have thread 1 calling FindLogEntries
and thread 2 calling a mocked http api.

Since the foreach loop above does not hold a lock, the log entry for the call made by thread 2 is added to the LogEntries property. This causes the enumerator in the foreach loop to throw an exception.

I hope that explanation makes sense?

[theramis]

Also in the DeleteLogEntry() method, it's safer to use the LogEntries property

Yup in that method LogEntries is perfect, since the Remove method on LogEntries is thread safe.

[StefH]

Thanks for this PR, can you take a look at my comments?

[gregoks]

hi guys,

I'm using version 1.0.30-ci11848 and still getting this error:

HttpStatusCode set to 500 System.InvalidOperationException: Collection was modified; enumeration operation may not execute. at System.Collections.Generic.List1.Enumerator.MoveNextRare() at System.Linq.Enumerable.WhereEnumerableIterator1.ToList() at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source) at WireMock.Owin.WireMockMiddleware.LogRequest(LogEntry entry, Boolean addRequest) at WireMock.Owin.WireMockMiddleware.InvokeInternal(HttpContext ctx) at WireMock.Owin.GlobalExceptionMiddleware.InvokeInternal(HttpContext ctx)

Is it the same as in this bug?

[StefH]

@gregoks A new fix is required I think.
I will check,.

[StefH]

@gregoks Can you please try:
WireMock.Net.1.0.31-ci-11865