Fixed deadlock in System.Resources.ResourceSet

[PashaPash]

Fixed deadlock between ResourceSet iterator and and ResourceSet.GetObject.

Using both ResourceSet iterator and ResourceSet.GetObject is a common scenario in web app - resource set is used for server view rendering (through ResourceSet.GetObject) and in the time that resource set is offloaded to client side as JSON (through ResourceSet iterator). There is a high probability that app will deadlock on start in that case.

Connect Issue: #580259 - closed as non-reproducable. I have no access to the source code in that connect issue, but deadlock graph seems to be exactly the same as in my sample app.

Sample app to reproduce the issue: https://github.com/PashaPash/ResManDeadlock

Details from Stack Exchange thread:

The problem is that when using a ResourceSet the iterator takes out locks on the internal resource cache of the ResourceReader http://referencesource.microsoft.com/#mscorlib/system/resources/resourcereader.cs,1389 and then in the method AllocateStringNameForIndex takes out a lock on the reader itself: http://referencesource.microsoft.com/#mscorlib/system/resources/resourcereader.cs,447

lock (_reader._resCache) {
     key = _reader.AllocateStringForNameIndex(_currentName, out _dataPosition); // locks the reader

Getting an object takes out the same locks int the opposite order:

http://referencesource.microsoft.com/#mscorlib/system/resources/runtimeresourceset.cs,300
and http://referencesource.microsoft.com/#mscorlib/system/resources/runtimeresourceset.cs,335

lock(Reader) {
    ....
    lock(_resCache) {
       _resCache[key] = resLocation;
    }
}

[PashaPash]

I updated the commit message to match contribution guidelines

[AlexGhiondea]

Thanks for the contribution. At first glance the change look ok.

However, since this code is shared with Desktop we need to run this through additional validation to make sure we don't break anything.

Once we are sure we are not breaking anything we are going to merge it in.

@ramarag can you take a look at this?

[ramarag]

Can you fix the indentation here. I am almost done running the tests. will merge when I get the updated PR
Also please do squash your commits to a single one

[PashaPash]

@ramarag, sorry for broken indentation, just tried to keep number of lines changed at minimum.

[PashaPash]

I added an indentation and rebased commit to master. Please review.

[ramarag]

LGTM... @jkotas can you take a look too.

[jkotas]

LGTM

[jkotas]

Thank you for getting this fixed!