Fix race condition in FileAccessRepository

[dfederm]

Fix race condition in FileAccessRepository

Today when we allow file accesses after project completion, we avoid disposing the log file stream and relevant to this bug we also avoid nulling out the fileTable and deletedDirectories. outside the lock we enumerate these collections to return a FileAccesses to the plugin. If a file access comes in while we're enumerating, we can get an exception like this:

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Enumerator.MoveNext()
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at Microsoft.MSBuildCache.FileAccess.FileAccessRepository.FileAccessesState.ProcessFileAccesses(Dictionary`2 fileTable, List`1 deletedDirectories) in D:\a\_work\1\s\src\Common\FileAccess\FileAccessRepository.cs:line 301
   at Microsoft.MSBuildCache.FileAccess.FileAccessRepository.FileAccessesState.FinishProject() in D:\a\_work\1\s\src\Common\FileAccess\FileAccessRepository.cs:line 275
   at Microsoft.MSBuildCache.MSBuildCachePluginBase`1.<HandleProjectFinishedInnerAsync>d__59.MoveNext() in D:\a\_work\1\s\src\Common\MSBuildCachePluginBase.cs:line 495
   at Microsoft.MSBuildCache.MSBuildCachePluginBase`1.<HandleProjectFinishedAsync>d__58.MoveNext() in D:\a\_work\1\s\src\Common\MSBuildCachePluginBase.cs:line 456

On solution would be to put ProcessFileAccesses inside the lock. However, upon thinking about it, I realized that we don't need to avoid nulling out those collections and we should just do it regardless. There are already null checks on these collections when they're added to, so there is safety from that aspect. For the data loss, we anyway only use those collections when the project is reported as complete, so it does not hurt anything to just avoid adding to the collections (by nulling them out) after the project finishes.

I am leaving the avoidance of the log file stream disposal though so we can still get the logging for these files. Also, I added a log line for the project finishing to have a clear delineation between accesses which end up being considered vs those which are missed in the logs.

[johnterickson]

Is this sufficient for the .NET memory model?