Delphi: preferred way of protection using Critical Sections

Question

I have an object x that needs to be accessed from several (5+ threads). The structure of the object is

    Tx = class
    private
      Fx: integer;
    public
      property x: integer read Fx read Fx;
   etc;

What is the better (most elegant) way of protection:

a)

Tx = class
 private
   Fx: integer;
 public
   property x: integer read Fx read Fx;
 public
   constructor Create; <--- Create the criticalsection here
   destructor Destroy; <--destroy it here
 etc;

var
   cs: TCriticalSection;
   Obj: Tx;

function GetSafeObject(): Tx;
begin
CS.Enter;
try
Result:= Obj;
finally
CS.Leave;

end;

and access always the object as GetSafeObj().x:= 3;

or

Tx = class
 private
   Fx: integer;
   FCS: TCriticalSection;
 public
   property x: integer read GerX read SetX;
 public
   constructor Create; <--- Create the criticalsection here
   destructor Destroy; <--destroy it here
 etc;

where 
 function Tx.Getx(): integer;
 begin
   CS.Enter;
   try
     Result:= Fx;
   finally
     CS.Leave;
   end;
end;

end;

and always access the object normally. I guess the first option is more elegant, even if both methods should work fine. Ay comments?

I would not use any of those two options, but rely on the `Lock/Unlock` methods (or make the critical section public), as proposed in TOndrej's answer. Option A would not protect anything. Option B will work with integer, but will fail if you return a class instance: if you make a copy of the property value (this is the case with an `integer` or even `string`), this is OK, but you shall `protect` all returned data if it is returned by reference. — Arnaud Bouchez, Nov 12 '11 at 09:48
Your question is ill posed. You need to describe how you plan to access the integer prop x — David Heffernan, Nov 12 '11 at 10:02

score 7 · Answer 1 · answered Nov 12 '11 at 01:49

Go for option B, making the critical section internal to the object. If the user of the class has to make use of an external function to safely access the instance, it is inevitable that somebody won't and the house will come tumbling down.

You also need to think about what operational semantic you are wanting to protect from multiple concurrent reads & writes. If you put a lock inside your getter and setter, you can guarantee that your object is internally coherent, but users of your object may see multithreading artifacts. For example, if thread A writes 10 to a property of your object, and thread B writes 50 to that property of the same object, only one of them can be last one in. If A happens to go first, then A will observe that they wrote a 10 to the property, but when they read it back again they see B's 50 that snuck in there in the gap between A's read-after-write.

Note also that you don't really need a lock to protect a single integer field. Aligned pointer sized integer writes are atomic operations on just about every hardware system today. You definitely need a lock to protect multi-piece data like structs or multi-step operations like changing two related fields at the same time.

If there is any way you can rework your design to make these objects local to a particular operation on a thread, do it. Making local copies of data may increase your memory footprint slightly, but it can dramatically simplify your code for multithreading and run faster than leaving mutex landmines all over the app. Look for other simplifying assumptions as well - if you can set up your system so that the object is immutable while its available to multiple threads, then the object doesn't need any lock protections at all. Read-only data is good for sharing across threads. Very very good.

Atomic operation does not equal thread safe. Depends on implementation. Read more [here](http://stackoverflow.com/questions/5481030/are-delphi-simple-types-thread-safe). — LU RD, Nov 12 '11 at 08:33
That is why Lock/UnLock methods does make sense - and why TOndrej's answer sounds better to me. — Arnaud Bouchez, Nov 12 '11 at 09:49
Atomic write does mean thread safe for the purpose of ensuring that no reader will ever read a partially written value. Atomic write is meaningless if you have multivalue semantics (multiple pieces of data that need to be updated in the same operation) and your design requires that all values be coherent with each other at all times. — dthorpe, Nov 12 '11 at 21:37
If the resource that needs protecting is the integer field, then an internal lock might be warranted - or might be completely unnecessary. If the resource that needs protecting (needs to be kept in a consistent state) is the entire object, then a check-out model to obtain exclusive access to the instance (like TThreadList) is more appropriate. PS, I wrote TThreadList. ;> — dthorpe, Nov 12 '11 at 21:45
@ArnaudBouchez The OP asked about thread safety with an integer field. If instead they are actually using a complex / multivalue type such as class instead of an integer field, then they asked the wrong question. — dthorpe, Nov 12 '11 at 21:50

score 6 · Answer 2 · answered Nov 12 '11 at 01:20

Making the CS be a member of the object and using the CS inside of property getter/setter methods is the correct approach. The other approach does not work because it locks and unlocks the CS before the object is actually acessed, so the property value is not protected at all.

score 5 · Answer 3 · answered Nov 12 '11 at 01:29

5

An easy way is to have a thread-safe wrapper around the object, similar to TThreadList. The wrapper needs two methods: Lock (to enter the critical section and return the inner object) and Unlock (to leave the critical section).

answered Nov 12 '11 at 01:29

Ondrej Kelle

36,941
2
65
128

+1 Even if such methods make the user code a bit more verbose (`Lock; try... finally Unlock; end;`), this is the best option for performance (especially if your object may be accessed safely before the multi-threading, or if you want to access to several properties of the objects), and also for race-condition identification with complex types (working with integer is OK, but if the property returns a class instance, the whole process would be better protected until you explicitelly do not use the object any more). – Arnaud Bouchez Nov 12 '11 at 09:43

Delphi: preferred way of protection using Critical Sections

3 Answers3

Linked