Initialized Lazy field in lambda not being reused among multiple threads

Question

I have method which is generating multiple threads (lambdas) in parallel and during its execution it access Lazy field property defined in class in which this method is invoked like:

class A {

    private Lazy<FabricClient> _fabricClient => new Lazy<FabricClient>(() => GetDefaultFabricClient());

    private FabricClient FabricClient => _fabricClient.Value;

    internal A() 
    {
         Console.WriteLine(FabricClient.ToString());
    }


    private void tempMethod()
    {

        List<String> listOfStrings = GetStrings();

        RunThreads(listOfStrings.Select<string, ThreadStart>(tempString => () =>
        {

            var x = FabricClient.GetServiceList();

        }).ToArray());

    }

    private FabricClient GetDefaultFabricClient()
    {
      // Environment is inherited property, I cannot edit it  
      // And it's defined like
      //
      // public Environment Environment
      // { get { return _context.Environment; }}
      // 
      if (Environment.IsPublicEnvironment)
      {
            return new FabricClient(Credentials, Endpoint);
      } 

      return new FabricClient();
    }

}

Is it possible to ensure that all threads would access same property, same object (as currently each thread is initializing its own FabricClient Lazy object, not reusing previous one being initialized, possibly not making it static)?

Also lazy FabricClient property is being populated before tempMethod execution, but it's not being reused in RunThreads method.

The mistake you have made here is using `=>`. See the duplicate for what it is doing, under the covers. — mjwills, Sep 03 '19 at 12:20
If you can't use `=` instead of `=>` then **don't assign the `Lazy` on declaration**. Instead, declare it as normal (i.e. just use `private Lazy _fabricClient;`) then assign it **in the constructor**. — mjwills, Sep 03 '19 at 14:16

canton7 · Accepted Answer · 2019-09-03T12:15:29.030

3

You've defined the _fabricClient property as:

private Lazy<FabricClient> _fabricClient => new Lazy<FabricClient>(() => {
    return new FabricClient();
});

This (due to the use of =>) says "every time _fabricClient is accessed, create a new Lazy". This is the opposite of what you want.

You want a normal field here, not a property (and you might as well make it readonly):

private readonly Lazy<FabricClient> _fabricClient = new Lazy<FabricClient>(() => {
    return new FabricClient();
});

This will create a single Lazy instance and store it in the _fabricClient field when an instance of A is constructed. Every time you access _fabricClient (through the FabricClient property), you will get the same Lazy instance.

To make this a little clearer, your _fabricClient property is the same as:

private Lazy<FabricClient> _fabricClient
{
    get
    {
        return new Lazy<FabricClient>(() => {
            return new FabricClient();
        });
    }
}

You can add some logging here, and see that the getter is executed every time this property is accessed:

private Lazy<FabricClient> _fabricClient
{
    get
    {
        Console.WriteLine("Constucting a new Lazy");
        return new Lazy<FabricClient>(() => {
            return new FabricClient();
        });
    }
}

edited Sep 03 '19 at 12:15

answered Sep 03 '19 at 12:08

canton7

37,633
3
64
77

I am always accessing via FabricClient property, so it should always fetch single Lazy instance, but it seems it does not? – luka032 Sep 03 '19 at 12:12
@luka032 "*I am always accessing via FabricClient property"* -- that bit's fine; "*so it should always fetch single Lazy instance*" -- there is no single Lazy instance for it to access! You create a new Lazy instance every time that `_fabricClient` is accessed, because `_fabricClient` is a property! – canton7 Sep 03 '19 at 12:13
Thanks for provided answer, it would work in normal circumstances, but I am dependent on other non-static fields, which I am accessing before return new FabricClient(); in the Lazy body, so I am bit stuck. – luka032 Sep 03 '19 at 13:31
@luka032 I don't see how that changes anything. Just try replacing `=>` with `=`, and tell me what specifically goes wrong – canton7 Sep 03 '19 at 13:45
1

@luka032 There is no call to `GetDefaultFabricClient()` in your updated code. Please post your *actual* code, not these made-up snippets which apparently have nothing to do with reality. – canton7 Sep 03 '19 at 14:16
Sorry, wanted to simplify code by replacing method call with its body. I have updated it now. – luka032 Sep 03 '19 at 15:06
@luka032 so simply instantiate the Lazy in your constructor: `private Lazy _fabricClient`, then in your constructor `_fabricClient = new Lazy(() => GetDefaultFabricClient());` – canton7 Sep 03 '19 at 15:10
2

@luka032 Your problem was that you couldn't instantiate your Lazy outside of your constructor. You thought the solution was to make it a property, then you hit problems. This is called an XY problem. When you hit problems, it's important that you ask for help with you *first* problem, before you try solutions which themselves don't work. Also make sure you post your actual code, otherwise we waste a lot of time trying to understand why we give you a solution and you refuse it (because of context you haven't given us) – canton7 Sep 03 '19 at 16:20

gmn · Answer 2 · 2019-09-03T14:19:50.407

-2

This is wrong, Lazy is threadsafe by default

You probably need some kind of lock around where the lazy gets used because all of those threads might reinitialise the lazy because they're probably all calling it before its been fully created.

Something like:

lock(syncObject)
{
   var client = FabricClient;
}
var x = FabricClient.GetServiceList();

where sync object is just a standard newed up clr object

object syncObject = new object();

edited Sep 03 '19 at 14:19

answered Sep 03 '19 at 11:58

gmn

4,199
4
24
46

Please note last note: Also lazy FabricClient property is being populated before tempMethod execution, but it's not being reused in RunThreads method. – luka032 Sep 03 '19 at 11:59
you should ammend your question code to reflect that @luka032 if you can – gmn Sep 03 '19 at 12:00
1

You don't need a lock when using `Lazy` - it defaults to `LazyThreadSafetyMode.ExecutionAndPublication` – canton7 Sep 03 '19 at 12:04
1

@canton7 you learn something new every day :). I'm leaving this answer here though, it might actually be useful even though its wrong. – gmn Sep 03 '19 at 13:11

Initialized Lazy field in lambda not being reused among multiple threads

2 Answers2