Adds Thread safe expression cache

[rducom]

Add lock on TryGetValue + separate locking object

[VitaliyMF]

@rducom could you provide some explanation for your changes? If this is a bugfix, please create an issue and describe how to reproduce it (ideally if you have simple unit test that illustrates the issue)

I have some questions:

1. why introduce separate __lock object?
2. is lock for CachedExpressions.TryGetValue call is necessary? As I know only iteration through dictionary is not a thread-safe.
3. Definitely condition for UseCache at line 88 is excessive: if (!UseCache || compiledExpr == null), it is enough to check for null.

[rducom]

It's not an issue, juste a code improvement.

1 - it's a best practice, and ensure we have no thread-lock 2 - CachedExpressions.TryGetValue is not thread-safe. In fact, no Dictionary<TKey,TValue> method call is thread-safe. Without the PCL constraint, we should have use ConcurrentDictionary<,> instead. 3 - You are right, I'll correct it

[VitaliyMF]

CachedExpressions.TryGetValue is not thread-safe. In fact, no Dictionary<TKey,TValue> method call is thread-safe.

In general you're right, but exactly in this usage Dictionary<>.TryGetValue call should be safe because we never remove anything from the cache, only new elements could be added. That's why I asked if this is a bugfix, I want to reproduce this issue and understand why my assumption about TryGetValue is wrong.

[rducom]

TryGetValue is thread safe with itself only (ie : you initialize the dictionary once, the only call TryGetValue). If you add values, this could result in unexpected behavior. (by design) So if you lock on the Add (via the indexer), then you have to lock on the TryGetValue method call.

Most of the time, the original code should be fine, but in some cases, this can result in unexpected problems.

[VitaliyMF]

Ok, it seems extra lock should not affect performance significantly so let it be :)

Thank you for your contribution!