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?

Answer 1

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.

Answer 2

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.