Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is there any way I can call this getter and consistently get the expected result?

I need to use a property getter from a 3rd-party API, and just accessing that getter sometimes hangs the entire application, sometimes works (in dev/debugger). And if I deploy it to a production server, the host application sometimes works as expected, sometimes dies with an APPCRASH in KERNEL32.

I couldn't understand how a simple getter could freeze up or crash anything, so I though maybe that property was a method in disguise, and used my favorite decompiler to find out.

This is the scheme/pattern I came across:

internal MyClass() : ISomeInterface
{
    Locker = new object();
}

public object Locker { get; private set; }

private SomeObject _myProperty;
public SomeObject MyProperty
{
    get
    {
        lock (Locker)
        {
            if (_myProperty== null)
                MyProperty = SomeCondition ? SomeMethod() : Something.Else;
            return _myProperty;
        }
    }

    private set
    {
        _myProperty = value;
    }
}

Am I correct to believe that whenever _myProperty is null, the getter calls the setter and there's a race condition bug here that may be causing the random/occasional freezing I'm experiencing? How can I confirm/infirm that this freezing is effectively a deadlock?

What would be a correct implementation? I'm led to believe that if the setter also had a lock (Locker) (I don't like the Locker being publicly accessible either, sounds like asking for trouble), and the getter would assign _myProperty instead of calling the setter, it'd be thread-safe.

Lastly, is there a way I can call this getter without requiring the 3rd party to ship a "fixed" version, and without freezing?

I have tried calling it from the main UI thread, I have tried calling it from a Task / background thread, I have tried a Sleep(200) before the call, I have tried setting a timeout on the Task... no matter what I do, I don't seem to be able to consistently call that getter and reliably get the expected result (it does work... sometimes).

The Locker object is public, but it's defined in an internal class that I only access via its interface, which of course doesn't expose it. Is there a way I can use reflection to acquire the lock from the outside (Monitor.TryEnter(magicallyObtainedLocker)) and actually get it to work? Or is that getter completely borked?

like image 319
Mathieu Guindon Avatar asked Jan 31 '14 20:01

Mathieu Guindon


2 Answers

Am I correct to believe that whenever _myProperty is null, the getter calls the setter and there's a race condition bug here that may be causing the random/occasional freezing I'm experiencing? How can I confirm/infirm that this freezing is effectively a deadlock?

No. In C#, lock is reentrant for the same thread. You can see this in the documentation for Monitor.Enter:

It is legal for the same thread to invoke Enter more than once without it blocking; however, an equal number of Exit calls must be invoked before other threads waiting on the object will unblock.


don't like the Locker being publicly accessible either, sounds like asking for trouble

I agree with you here. This is more likely the culprit. The fact that Locker is public most likely means that something else in the library is locking on this object, which could be the cause of the deadlock.

If you can run this through the Visual Studio Concurrency Profiler, you should be able to pause at the deadlock, and see the objects blocking at that point in time.

like image 187
Reed Copsey Avatar answered Nov 15 '22 15:11

Reed Copsey


Is there a way I can use reflection to acquire the lock from the outside (Monitor.TryEnter(magicallyObtainedLocker)) and actually get it to work?

Yes, given that you could decompile it, I would guess so1; the following 'works on my machine':

  • "ThirdParty" is built in its own, separate project as a DLL
  • "InternalLocker" is a console program in a different project, which references the ThirdParty DLL

1 I'm guessing you can acquire the lock from the outside; and hoping but not predicting whether that will suffice to avoid the deadlock.


using System;

namespace ThirdParty
{
    public interface ISomeInterface
    {
        string MyProperty { get; }
    }

    public static class Factory
    {
        public static ISomeInterface create() { return new MyClass(); }
    }

    internal class MyClass : ISomeInterface
    {
        internal MyClass()
        {
            Locker = new object();
        }

        public object Locker { get; private set; }

        private string _myProperty;
        public string MyProperty
        {
            get
            {
                lock (Locker)
                {
                    if (_myProperty == null)
                        MyProperty = "foo";
                    return _myProperty;
                }
            }

            private set
            {
                _myProperty = value;
            }
        }
    }
}

using System;
using System.Linq;

using System.Reflection;
using System.Threading;
using ThirdParty;

namespace InternalLocker
{
    class Program
    {
        static void Main(string[] args)
        {
            test1();
            test2();
        }

        static void test1()
        {
            ISomeInterface thing = Factory.create();
            string foo = thing.MyProperty;
            assert(foo == "foo");
        }

        static void test2()
        {
            ISomeInterface thing = Factory.create();
            // use reflection to find the public property
            Type type = thing.GetType();
            PropertyInfo propertyInfo = type.GetProperty("Locker");
            // get the property value
            object locker = propertyInfo.GetValue(thing, null);
            // use the property value
            if (Monitor.TryEnter(locker))
            {
                string foo = thing.MyProperty;
                Monitor.Exit(locker);
                assert(foo == "foo");
            }
            else
                assert(false);
        }

        static void assert(bool b)
        {
            if (!b)
                throw new Exception();
        }
    }
}
like image 40
ChrisW Avatar answered Nov 15 '22 17:11

ChrisW