0

actually what I'm simply trying to achieve is to get to know multithreading in C#.

SO i have this class called WeakeningEvictionary{TKey, TValue}, which has a private Dictionary{TKey, CachedValue{TValue}} that functions as the cache. CachedValue is a Wrapper that has a Strong- and WeakReference to TValue. After a predefined Time a Task is created to nullify the StrongReference and put it into WeakReference. I also have a HashSet implemented that keeps track of which keyValuePairs to evict. (added to when weakening happened, removed from when SetValue is called) Immediately after GC has done its Job another Task is created to evict all those mentioned Pairs.

Actually I wouldn't need a RecursiveLock for this, but I encountered Issues, when some stored Information is asked recursively because a construction series required so.

So I came up with this code: (Updated, was a not-going-to-work ExtensionMethod before)

    public void RecursiveEnter(Action action)
    {
        if (_spinLock.IsHeldByCurrentThread)
        {
            action();
        }
        else
        {
            bool gotLock = false;
            _spinLock.Enter(ref gotLock);//blocking until acquired
            action();
            if (gotLock) _spinLock.Exit();
        }
    }

So what I'm trying to do now is:

    private void Evict()
    {
        RecursiveEnter(() =>
            {
                foreach (TKey key in toEvict)
                {
                    _dict.Remove(key);
                }
            }
        );
    }

Alright what if I use And my Question is: What are the Risks? And are Closures known to cause Issues when being used by Threads in this way?

Thanks for your Input ;-)

Christoph Wolf
  • 95
  • 1
  • 12

1 Answers1

1

Right off the bat, the method call is 100% not going to work: SpinLock is a value type, you must pass it by reference (RecursiveEnter(ref SpinLock spinLock, Action action)) and not by value.

See for example https://learn.microsoft.com/en-us/dotnet/api/system.threading.spinlock?view=netframework-4.7.2#remarks

I'm not sure this is the best thing for you to use: you should start with a higher-level primitive (maybe a ReaderWriterLockSlim) and refine things only with careful testing and understanding.

Mark Sowul
  • 10,244
  • 1
  • 45
  • 51
  • ha, alright nice, thanks ;-) You know, some people just like tinkering ;-) – Christoph Wolf Oct 03 '18 at 20:00
  • But what if I make it call the field _spinLock, like (going to be updated right now) above? Going to check out the ReaderWriterLockSlim, btw ;-) – Christoph Wolf Oct 03 '18 at 20:02
  • Tinkering is great, but multithreading and locks are rich sources of one-in-a-million bugs so you have to be very careful and should probably read a bit more. There's a reason why you're having to fight so hard to do what you want with `SpinLock`: it's not really designed for your scenario – Mark Sowul Oct 03 '18 at 20:06
  • Two other issues are that the spin lock doesn't necessarily track `IsHeldByCurrentThread` unless it was specifically created to do so (https://learn.microsoft.com/en-us/dotnet/api/system.threading.spinlock.isheldbycurrentthread?view=netframework-4.7.2) and you have to check the value `gotLock` after calling Enter because you may not have actually acquired it successfully (https://learn.microsoft.com/en-us/dotnet/api/system.threading.spinlock.enter?view=netframework-4.7.2) so you shouldn't just call `action()` without checking – Mark Sowul Oct 03 '18 at 20:13
  • @ChristophWolf yes you're right and I've deleted that part – Mark Sowul Oct 03 '18 at 20:15
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/181250/discussion-between-christoph-wolf-and-mark-sowul). – Christoph Wolf Oct 03 '18 at 20:18
  • First issue: is enabled ;-) Second Issue: https://stackoverflow.com/questions/33165323/how-can-spinlock-enter-fail-to-acquire-the-lock Any Ideas about that Closure? – Christoph Wolf Oct 03 '18 at 20:36
  • 1
    I don't think there's anything wrong with passing in an action per se, however you should protect the lock with a try/finally. Do note though that my testing of the MSDN sample showed that a regular lock was faster than the SpinLock. – Mark Sowul Oct 03 '18 at 21:12