C # thread security with get / set

Question

This is a detailed question for C #.

Suppose I have a class with an object, and this object is protected by a lock:

Object mLock = new Object(); MyObject property; public MyObject MyProperty { get { return property; } set { property = value; } } 

I want the poll thread to be able to request this property. I also want the thread to periodically update the properties of this object, and sometimes the user can update this property, and the user wants to see this property.

Will the following code correctly block data?

 Object mLock = new Object(); MyObject property; public MyObject MyProperty { get { lock (mLock){ return property; } } set { lock (mLock){ property = value; } } } 

"Right," I mean, if I want to call

 MyProperty.Field1 = 2; 

or something else, will the field be locked during the update? Is the setup performed by the equals statement inside the scope of the get function, or does the get function (and therefore the lock) end first, and then the setup is called, and then set, bypassing the lock?

Edit: since this apparently won't do the trick, what will happen? I need to do something like:

 Object mLock = new Object(); MyObject property; public MyObject MyProperty { get { MyObject tmp = null; lock (mLock){ tmp = property.Clone(); } return tmp; } set { lock (mLock){ property = value; } } } 

which more or less simply ensures that I only have access to the copy, which means that if I had two streams calling "get" at the same time, each of them would start with the same Field1 value (on the right ?). Is there a way to make reading and writing a lock on a property that makes sense? Or should I just limit myself to locking on function sections, not the data itself?

Just to make this example make sense: MyObject is a device driver that returns status asynchronously. I send commands through the serial port, and then the device responds to these commands in due time. Right now I have a thread that polls it for its status ("Are you still there? Can you accept commands?"), A thread waiting for replies to the serial port ("Just received status bar 2, all is good")) and then a UI thread that accepts other commands ("The user wants you to do this.") and sends the responses from the driver ("I just did, now update the interface with this"). Therefore, I want to block the object itself, not the fields of the object; this would be a huge number of locks, a and b, not all devices of this class have the same behavior, just common behavior, so I would have to code many separate dialogs if I individualized the locks.

Answer 1

No, your code will not block access to members of the object returned from MyProperty . It only blocks MyProperty .

Your example uses two operations collapsed into one, roughly equivalent to this:

 // object is locked and then immediately released in the MyProperty getter MyObject o = MyProperty; // this assignment isn't covered by a lock o.Field1 = 2; // the MyProperty setter is never even called in this example 

In a nutshell - if two threads access MyProperty at the same time, the receiver briefly blocks the second thread until it returns the object to the first thread, but then returns the object to the second thread. Both threads will have full, unlocked access to the object.

EDIT in response to further details in the question

I'm still not 100% sure what you are trying to achieve, but if you just want to get atomic access to the object, then you will not have blocking the calling code against the object itself?

 // quick and dirty example // there almost certainly a better/cleaner way to do this lock (MyProperty) { // other threads can't lock the object while you're in here MyProperty.Field1 = 2; // do more stuff if you like, the object is all yours } // now the object is up-for-grabs again 

Not ideal, but as long as all access to the object is contained in lock (MyProperty) sections, then this approach will be thread safe.

Answer 2

Parallel programming will be quite simple if your approach can work. But this is not the case, an iceberg that is drowning that the Titanic, for example, a client of your class does this:

 objectRef.MyProperty += 1; 

The read-modify-write race is pretty obvious, there are worse ones. There is absolutely nothing you can do to make your property stream safe, except to make it immutable. This is your client who needs to cope with a headache. Being forced to delegate such responsibility to a programmer who is the least credible, this is the Achilles heel of parallel programming.

Answer 3

As others have pointed out, once you return an object from a getter, you lose control over who is accessing the object and when. To do what you want to do, you need to put a lock inside the object itself.

Perhaps I do not understand the whole picture, but based on your description, it does not look like you definitely need to have a lock for each individual field. If you have a set of fields that are simply read and written through getters and setters, you can probably get away with a single padlock for these fields. Obviously, it is likely that you needlessly serialize your threads in this way. But then again, based on your description, it doesn't look like you are also actively accessing the object.

I also suggest using an event instead of using a thread to poll the status of a device. Using the polling mechanism, you will block the lock every time a thread requests a device. With an event engine, as soon as the status changes, the object will notify listeners. At this point, your poll stream (which will no longer be polling) wakes up and gets a new status. It will be much more efficient.

As an example...

 public class Status { private int _code; private DateTime _lastUpdate; private object _sync = new object(); // single lock for both fields public int Code { get { lock (_sync) { return _code; } } set { lock (_sync) { _code = value; } // Notify listeners EventHandler handler = Changed; if (handler != null) { handler(this, null); } } } public DateTime LastUpdate { get { lock (_sync) { return _lastUpdate; } } set { lock (_sync) { _lastUpdate = value; } } } public event EventHandler Changed; } 

Your survey stream will look something like this.

 Status status = new Status(); ManualResetEvent changedEvent = new ManualResetEvent(false); Thread thread = new Thread( delegate() { status.Changed += delegate { changedEvent.Set(); }; while (true) { changedEvent.WaitOne(Timeout.Infinite); int code = status.Code; DateTime lastUpdate = status.LastUpdate; changedEvent.Reset(); } } ); thread.Start(); 

Answer 4

The lock area in your example is in the wrong place - it should be in the property area of ​​the MyObject class, not the container.

If MyObject my object class is simply used to store data that one stream wants to write and another (user interface stream) to read, then you may not need a setter at all and build it once.

We also consider whether the placement of locks at the property level is the level of the lock record; if more than one property can be written to represent the state of a transaction (for example, total orders and total weight), it would be better to block the MyObject level (for example, lock (myObject.SyncRoot)) ..)

Answer 5

In the code example you provided, retrieval is never performed.

In a more complex example:

 MyProperty.Field1 = MyProperty.doSomething() + 2; 

And of course, assuming you did:

 lock (mLock) { // stuff... } 

In doSomething() then all lock calls will not be sufficient to guarantee synchronization across the entire object. As soon as the doSomething() function returns, the lock will be lost, then the addition will be completed, and then the assignment will happen, which will close again.

Or, to write it another way, you can pretend that locks are not performed amutomatically and rewrite it more like "machine code" with one operation per line, and it becomes obvious:

 lock (mLock) { val = doSomething() } val = val + 2 lock (mLock) { MyProperty.Field1 = val } 

Answer 6

The beauty of multithreading is that you do not know in what order events will occur. If you install something in one thread, this may happen first, this may happen after receiving.

The code you posted blocked the item while it was reading and writing. If you want to handle the case when the value is updated, you might need to explore other forms of synchronization, such as events . (Check auto / manual versions). Then you can tell your “polling” thread that the value has changed and it will be re-read.

Answer 7

In your edited version, you still do not provide a streaming update mode for MyObject. Any changes to the properties of the object must be made inside the synchronization / lock block.

You can write separate setters to handle this, but you indicated that it will be difficult due to the large number of fields. If this is true (and you have not yet given enough information to evaluate this), one option is to write a setter that uses reflection; this will allow you to pass a string representing the field name, and you can dynamically search for the field name and update the value. This will allow you to have one setter that will work on any number of fields. It is not so simple or effective, but it will allow you to deal with a large number of classes and fields.

Answer 8

You implemented a lock to receive / set the object, but you did not make the stream of objects safe, which is a different story.

I wrote an article about immutable classes of models in C # that may be interesting in this context: http://rickyhelgesson.wordpress.com/2012/07/17/mutable-or-immutable-in-a-parallel-world/

Answer 9

C # locks don't suffer from the same lock problems as other languages?

eg.

 var someObj = -1; // Thread 1 if (someObj = -1) lock(someObj) someObj = 42; // Thread 2 if (someObj = -1) lock(someObj) someObj = 24; 

This can lead to the fact that the problem of both threads will ultimately lead to their blocking and change of value. This can lead to some strange errors. However, you do not want to unnecessarily block the object if you do not need to. In this case, you should consider double check locking.

 // Threads 1 & 2 if (someObj = -1) lock(someObj) if(someObj = -1) someObj = {newValue}; 

Just something to keep in mind.