Instance instance sets static member, is it safe?

Question

I will refactor some code and wonder about using lock in the instance constructor.

 public class MyClass { private static Int32 counter = 0; private Int32 myCount; public MyClass() { lock(this) { counter++; myCount = counter; } } } 

Please confirm

• Instance constructors are thread safe.
• The lock statement prevents access to this block code, and not to the static counter member.

If the intention of the original programmer was to let each instance know its own “account”, how could I synchronize access to the “counter” member to ensure that the other thread is not new to MyClass and changes the count before setting its account?

FYI - This class is not single. Instances should simply be aware of their quantity.

Answer 1

@ajmastrean

I'm not saying that you should use the singleton template itself, but accept its method of encapsulating the instantiation process.

i.e.

• Make the constructor private.
• Create a static instance method that returns a type.
• In the static instance method, use the lock keyword before creating the instance.
• Create a new instance of the type.
• Increase counter.
• Unlock and return the new instance.

EDIT

One problem that occurred to me if you know when the account came down ?;)

CHANGE AGAIN

Thinking about this, you can add code to the destructor, which calls another static method to decrease the counter: D

Answer 2

If you only increase the number, there is a special class (lock) just for that ...

http://msdn.microsoft.com/en-us/library/system.threading.interlocked.increment.aspx

Interconnected method.

Increases the given variable and saves the result as an atomic operation.

 System.Threading.Interlocked.Increment(myField); 

More information on thread renaming methods ...

http://msdn.microsoft.com/en-us/library/1c9txz50.aspx

Answer 3

I guess this is for a singleton pattern or something like that. What you want to do is not to lock your object, but to lock the counter while you modify it.

 private static int counter = 0; private static object counterLock = new Object(); lock(counterLock) { counter++; myCounter = counter; } 

Because your current code is redundant. Especially in a constructor, where there is only one thread that the constructor can call, unlike methods, where it can be split into threads and accessible from any thread that is shared.

From a small number that I can tell from your code, you are trying to give the object a current account during its creation. Thus, with the above code, the counter will be locked until the counter is updated and installed locally. Thus, all other designers will have to wait for the counter to exit.

Answer 4

You can use another static object to lock it.

 private static Object lockObj = new Object(); 

and lock this object in the constructor.

 lock(lockObj){} 

However, I'm not sure if there are situations that need to be handled due to optimizing the compiler in .NET , as is the case with java

Answer 5

The most efficient way to do this is to use a lock operation with a lock. It will increment the counter and return the newly set value of the static counter at the same time (atomically)

 class MyClass { static int _LastInstanceId = 0; private readonly int instanceId; public MyClass() { this.instanceId = Interlocked.Increment(ref _LastInstanceId); } } 

In your original example, the lock (this) statement will not have the desired effect, because each individual instance will have a different "this" link, and therefore multiple instances can update the static member at the same time.

In a sense, constructors can be considered thread-safe because the reference to the object that is being built is not visible until the constructor completes, but it does not benefit from protecting the static variable.

(First, Mike Shull had a blocked bit)

Answer 6

I think that if you change the Singleton Pattern to include an account (obviously using a safe thread), you'll be fine :)

Edit

The shit that I accidentally deleted!

I'm not sure that ARE instance constructors are thread safe, I remember reading about this in the design template book, you need to make sure that the locks are in place during the creation process, because of this.

Answer 7

@ Rob

Fyi. This class may not be single, I need access to different instances. They should just maintain an account. What part of the singleton pattern would you change to increment the counter?

Or do you offer me a static method for building code blocking access, which increments and reads a counter with a lock.

 public MyClass { private static Int32 counter = 0; public static MyClass GetAnInstance() { lock(MyClass) { counter++; return new MyClass(); } } private Int32 myCount; private MyClass() { myCount = counter; } }