13

Is there any way to detect that a certain method in my code is called without using any lock in any of the methods below in the call stack?
The goal is to debug a faulty application and find out if certain pieces of code aren't thread safe.

user626528
  • 13,999
  • 30
  • 78
  • 146
  • ReSharper finds situations where a member variable is used sometimes inside a lock and sometimes outside. – Bernhard Hiller Oct 21 '16 at 08:07
  • @Bernhard Hiller, Resharper's abilities in this area are very, very limited. It's based on static analysis that catches only a limited subset of cases. – user626528 Oct 21 '16 at 09:49
  • You want to be able to do that without any modifications, using just regular "lock (object)" statements? I can imagine you can do that using some object wrappers which in debug mode will provide you with necessary information. – Evk Oct 26 '16 at 14:00
  • @Evk, ability to do that without modifying all the locking code would be great, yes. – user626528 Oct 27 '16 at 03:52
  • I suppose you just cannot do that if you use simple locks, unless you have a reference to an object which you are lock on. Arbitraty code can have or not have locks on arbitrary objects up the call stack, and you cannot know that. If you _do_ know the object on which lock should be held, you can use simple Monitor.TryEnter to check if it's locked. Otherwise, you will have to use some lock wrappers everywhere. – Evk Oct 27 '16 at 14:50
  • 2
    If you have access to the object used for locking (some property similar to `ICollection.SyncRoot`), then you can use [`Monitor.IsEntered`](https://msdn.microsoft.com/en-us/library/system.threading.monitor.isentered(v=vs.110).aspx) method. Otherwise it's impossible. – Ivan Stoev Oct 29 '16 at 17:17
  • I am not sure how extensive is your code, but you can simulate a multi-thread scenario and see if your code breaks. You can add unit tests to your classes/methods and validate thread safety. See the following article: https://jevgeni.net/2013/07/31/unit-testing-thread-safety-c/ – Charles Oct 31 '16 at 20:07
  • @Ivan Stoev, why don't make this an answer? – user626528 Nov 17 '16 at 08:10

2 Answers2

1

This seems like a decent use case for AOP (aspect oriented programming). A very basic summary of AOP is that its a method of dealing with cross cutting concerns to make code dry and modular. The idea is that if you're doing something to every method call on an object (eg. logging each call) instead of adding a log at the start and end of each method you instead you inherit the object and do that outside of the class as to not muddy its purpose.

This can be done a few ways and I'll give you an example of two. First is manually (this isn't great but can be done very easily for small casses).

Assume you have a class, Doer with two methods Do and Other. You can inherit from that and make

public class Doer
{
    public virtual void Do()
    {
        //do stuff.
    }

    public virtual void Other()
    {
        //do stuff.
    }
}

public class AspectDoer : Doer
{
    public override void Do()
    {
        LogCall("Do");
        base.Do();
    }

    public override void Other()
    {
        LogCall("Other");
        base.Other();
    }

    private void LogCall(string method)
    {
       //Record call 
    }
}

This is great if you only care about one class but quickly becomes unfeasible if you have to do it for many classes. For those cases I'd recommend using something like the CastleProxy library. This is a library which dynamically creates a proxy to wrap any class you want. In combination with an IOC you can easily wrap every service in your application.

Here's a quick example of using CastleProxy, main points being use ProxyGenerator.GenerateProxy and pass in IInterceptors to do stuff around method calls:

    [Test]
    public void TestProxy()
    {
        var generator = new ProxyGenerator();
        var proxy = generator.CreateClassProxy<Doer>(new LogInterceptor());
        proxy.Do();
        Assert.True(_wasCalled);
    }

    private static bool _wasCalled = false;
    public class LogInterceptor : IInterceptor
    {
        public void Intercept(IInvocation invocation)
        {
            Log(invocation.Method.Name);
            invocation.Proceed();
        }

        private void Log(string name)
        {
            _wasCalled = true;
        }
    }

Now, the logging portion. I'm not sure you really NEED this to be lockless, short locks might be enough but lets proceed thinking you do.

I don't know of many tools in C# that support lock free operations but the the simplest version of this I can see is using Interlocked to increment a counter of how many instances are in the method at any given time If would look something like this:

        [Test]
    public void TestProxy()
    {
        var generator = new ProxyGenerator();
        var proxy = generator.CreateClassProxy<Doer>(new LogInterceptor());
        proxy.Do();
        Assert.AreEqual(1, _totalDoCount);
    }

    private static int _currentDoCount = 0;
    private static int _totalDoCount = 0;
    public class LogInterceptor : IInterceptor
    {
        public void Intercept(IInvocation invocation)
        {
            if (invocation.Method.Name == "Do")
            {
                var result = Interlocked.Increment(ref _currentDoCount);
                Interlocked.Increment(ref _totalDoCount);
                if(result > 1) throw new Exception("thread safe violation");
            }


            invocation.Proceed();
            Interlocked.Decrement(ref _currentDoCount);
        }
    }

Interlocked uses magical register magic to do thread safe operation (Compare-And-Swap I believe, but I don't really know). If you need more context than just "It Happened". You can use a concurrent stack or a concurrent queue which are lockless (they use interlock as well: https://msdn.microsoft.com/en-us/library/dd997305.aspx/). I would include a timestamp on these though, since I haven't used them enough to know if they promise to return elements in the order they occurred.

Like I said above, you might not NEED lock free operations but this should. I don't know if any of this is a perfect fit for you since I don't know your exact problem but it should provide you some tools to tackle this.

JCalder
  • 391
  • 3
  • 8
0

You could host the CLR yourself, and track the locks taken using the IHostSyncManager::CreateMonitorEvent method. You'd then need to expose your own mechanism from your host to your method called say "IsLockTaken()". You could then call that from your method in your actual code.

I think it is possible, but it would be quite a lot of work and almost certainly a complete distraction from the problem you're trying to solve, but no doubt a lot of fun!

Here's an interesting read on Deadlock detection https://blogs.msdn.microsoft.com/sqlclr/2006/07/25/deadlock-detection-in-sql-clr/

Daniel James Bryars
  • 4,429
  • 3
  • 39
  • 57