1

i am a little confused by multi-thread access risk on a static property in C#.

public class MyClass
{
    public static MyClass Static
    {
        get
        {
            var c = new MyClass();
            c.SomeProperty = "12345";
            c.OtherProperty = DateTime.Now.ToString();
            return c;
        }
    }
}

This example class provides a static property that create a new instance of MyClass,

like a method:

public class MyClass
{
    public static MyClass Static()
    {
        var c = new MyClass();
        c.SomeProperty = "12345";
        c.OtherProperty = DateTime.Now.ToString();
        return c;
    }
}

Obviously, this property is not a "storage" box for an instance of MyClass, but it behaves like a static method (that, if i reading good a msdn article, is completely thread-safe).

My question is: i risk something with using this concept ? Especially in a web or multi-thread enviroinment ?

There is a no particular utility in using it, only for simple reading and cleaning code:

MyClass.Static.SomeProperty = "Something";

is more clear than

MyClass.Static().SomeProperty = "Something";

All help will be appreciated

Thanks

Servy
  • 193,745
  • 23
  • 295
  • 406
T-moty
  • 2,411
  • 21
  • 30
  • Take a look at some of the answers here: http://stackoverflow.com/questions/4026785/how-do-static-properties-work-in-an-asp-net-enviroment – asawyer May 30 '13 at 15:55
  • Static methods are mostly just really bad for testability and reusability, static _data_ is what is bad for pretty much everything. – Joachim Isaksson May 30 '13 at 15:59
  • The returned instance is a new instance every time you call Static, there is no threading issue there, but I wonder why you ask. – Casperah May 30 '13 at 15:59
  • @JoachimIsaksson: Static methods are fine, so long as you don't modify static state with them and expect them to be idempotent. In many ways, they are ideal for testing, if you write them in a functional style. I really don't understand why people think they are bad for testing. – Robert Harvey May 30 '13 at 16:10
  • @RobertHarvey They don't fit well into a lot of testing *frameworks*. It's not so much that they're hard to test in general, they just don't fit the paradigm of most(?) frameworks. – Servy May 30 '13 at 16:12
  • @Servy How tough could it be? You hand the method a value, and assert the return value. – Robert Harvey May 30 '13 at 16:12
  • @RobertHarvey You can't create a mock version of it when it's a dependency of another function. – Servy May 30 '13 at 16:14
  • @Servy: Why would you need to mock it? It's a function, with no external dependencies except the ones you pass to it (if you're writing it correctly). There's nothing to mock. – Robert Harvey May 30 '13 at 16:16
  • @RobertHarvey It's not that the method is hard to debug, it's that the "StaticWriteTheFile" method can't be mocked to a null/check-state method when testing classes _depending_ on it. – Joachim Isaksson May 30 '13 at 16:18
  • I'll just leave this here: http://stackoverflow.com/questions/5864076/mocking-static-methods – Robert Harvey May 30 '13 at 16:22
  • @RobertHarvey I did not say it was impossible to do, it's just much more effort to stub `DateTime.Now` than `protected virtual DateTime GetNow()`. For reusability, a `public virtual decimal GetTax(decimal amount)` is easier to override than `TaxCalculator.GetTax(decimal amount)` when you start selling in the next state. All possible, just more effort involved. – Joachim Isaksson May 30 '13 at 16:29
  • @JoachimIsaksson you are right: static methods are the way for unscalability of code, and lot of other problems. But, in this scenario, i need to use it for simplify and do my work fast. Anyway thanks for the clarification – T-moty May 30 '13 at 17:14
  • Are you really just looking for a Singleton? http://csharpindepth.com/articles/general/singleton.aspx – Jim Mischel May 30 '13 at 20:21
  • @JimMischel no. My property behaves like a method, and release an instance not accessible by other threads/sessions. Singleton is accessible by all threads. – T-moty May 30 '13 at 21:05
  • @JoachimIsaksson if you write 2 lines of "official" answer, i give you "the green V" – T-moty May 31 '13 at 06:14
  • possible duplicate of [Should C# methods that \*can\* be static be static?](http://stackoverflow.com/questions/731763/should-c-sharp-methods-that-can-be-static-be-static) – chollida Jun 02 '13 at 14:33

3 Answers3

2

It seems that you are creating a static factory method that will give you a fully instantiated object.

Threading would not be an issue here because every time you call this method or property you are creating a new object. If 2 threads call the same method at the same time they will each keep on working on the object they are dealing with

Having said that - Perhaps you should reexamine how you are using the class - because if you call this in your code

MyClass.Static.SomeProperty = "Something";

You are basically throwing away the object after it has been instantiated you would need to assign it to a variable and store it. - the next time you call that function you will receive a new object.

bluish
  • 23,093
  • 23
  • 110
  • 171
  • Yes, you understand perfectly my question. In fact, i need to use it with a hidden factory method: instance new object or reuse if the current session/thread already have an instance – T-moty May 30 '13 at 17:01
  • If you mean session then your best bet would be to use LAZY, http://stackoverflow.com/questions/6847721/when-to-use-lazyt-in-c-sharp – WantToBeAnonomous May 31 '13 at 17:03
2

In both your examples you're returning a new instance of MyClass every time the property is accessed. There is no danger that you'll have any concurrency issues when multiple threads access the static property method at the same time, because they're actually modifying the properties of their own instance of MyClass and not sharing it between them.

If you had something like this instead:

public class MyClass
{
    private static MyClass _myClass;
    public static MyClass Static
    {
        get
        {
            return _myClass ?? (_myClass = new MyClass());
        }
    }
}

...then you'd cause problems when two threads attempted to write/read properties of the resulting MyClass instance, because they're operating on the same MyClass reference _myClass.

Even so, there are two issues with the code you've posted:

You need to change it to a method and rename it, because it's actually creating something, not accessing a static version of anything. Then you can operate on the return value. Something like this:

public class MyClass
{
    public static MyClass Create()
    {
        var c = new MyClass();
        c.SomeProperty = "12345";
        c.OtherProperty = DateTime.Now.ToString();
        return c;
    }
}

Then use it like this:

var myClass = MyClass.Create();
myClass.SomeProperty = "Stuff";

The way you're setting properties currently means their values aren't persisted, because a new MyClass is created the next time the Static property is accessed.

If when you set SomeProperty you actually want a static instance to be updated you'll need to lock on a static object to solve the multi threading issue - something like this:

public static class MyClass
{
    private static readonly object locker = new object();

    private static string someProperty;

    public void SetSomeProperty(string val)
    {
        lock (locker)
        {
             someProperty = val;
        }
    }

    public void GetSomeProperty()
    {
        lock (locker)
        {
             return someProperty;
        }
    }
}
greg84
  • 7,225
  • 2
  • 30
  • 40
  • Maybe i can explain better: the property must work like a Factory, and instantiate new object if the session/thread not already contains an active instance. In this case, the static property is thread-safe? – T-moty May 30 '13 at 17:07
  • 1
    @T-moty That would depend how you write it. Per thread data is generally not a good idea in ASP.NET, per session should work. – Joachim Isaksson May 30 '13 at 17:51
  • @JoachimIsaksson of course: in ASP.NET by session and in console/winforms/etc by thread data. Good! – T-moty May 30 '13 at 18:28
0

Perhaps I was not able to explain properly, my question was referred multithreaded access on static properties.

I can confirm that they are thread-safe, if the returned object is bound to the current thread.

Here is the example of what I implemented:

public interface IObjectFactory
{
    T CreateOrReuse<T>() where T : class, new();
    T CreateOrReuse<T>(string key) where T : class, new();
    T CreateOrReuse<T>(string key, params object[] args) where T : class;
}

public class ThreadObjectFactory : IObjectFactory
{
    // implementation to create and store into the Thread Data
}

public class HttpSessionObjectFactory : IObjectFactory
{
    // implementation to create and store into the current session
}

Now the singleton:

public class MyClass
{
    public int PageLoadCounter = 0;

    public static MyClass Static
    {
        get
        {
            IObjectFactory factory = new HttpSessionObjectFactory();
            return factory.CreateOrReuse<MyClass>();
        }
    }
}

And this is the final use:

public class _Default : System.Web.UI.Page
{
    protected void Page_Load(object sender, EventArgs e)
    {
        MyClass.Static.PageLoadCounter++;
    }
}

Thanks for the replies, even if the question was not very clear

T-moty
  • 2,411
  • 21
  • 30