Is this the right way to thread-protect an object?

Question

I wrote an FAQ on a third-party website which pertains to thread-protecting objects in Delphi. What I'd like to know is if this thread-protection approach is accurate, or if I should change anything about it. I don't intend to ask about the FAQ in general, just the aim at the actual topic of multi-threading.

This code comes from common practices of mine, and I'm starting to wonder if these practices are good or bad. I've used this TLocker object for a long time, and would like to know if there's anything wrong with any of this approach.

The FAQ content:

There are many ways to thread-protect objects, but this is the most commonly used method. The reason you need to thread-protect objects in general is because if you have more than one thread which needs to interact with the same object, you need to prevent a deadlock from occuring. Deadlocks cause applications to freeze and lock up. In general, a deadlock is when two different threads try to access the same block of memory at the same time, each one keeps repeatedly blocking the other one, and it becomes a back and forth fight over access.

Protection of any object(s) begins with the creation of a Critical Section. In Delphi, this is TRTLCriticalSection in the Windows unit. A Critical Section can be used as a lock around virtually anything, which when locked, no other threads are able to access it until it's unlocked. This ensures that only one thread is able to access that block of memory at a time, all other threads are blocked out until it's unlocked.

It's basically like a phone booth with a line of people waiting outside. The phone its self is the object you need to protect, the phone booth is the thread protection, and the door is the critical section. While one person is using the phone with the door closed, the line of people has to wait. Once that person is done, they open the door and leave and the next comes in and closes the door. If someone tried to get in the phone booth while someone's using the phone, well, it's obvious that would lead to a fight. That fight would be a deadlock.

It is still a risk to cause a deadlock even when using this method, for example, someone trying to use the phone when someone else is using it. If for any reason a thread attempts to access the object without locking it, and while another thread is accessing it, you will still run into issues. So, you need to use this protection EVERYWHERE that could possibly need to access it. Remember, when you lock this, nothing else is able to lock it again (under the promise that all other attempts would also be using this same Critical Section). The locking thread must unlock it before it can be locked again.

Okay, so into the code. The four calls you will need to know are:

• InitializeCriticalSection() - Creates an instance of a Critical Section
• EnterCriticalSection() - Engages the lock
• LeaveCriticalSection() - Releases the lock
• DeleteCriticalSection() - Destroys an instance of a Critical Section

First, you need to declare a Critical Section somewhere. It's usually something that would be instantiated for the entire duration of the application, so I'll assume in the main form.

  TForm1 = class(TForm)
  private
    FLock: TRTLCriticalSection;

Then, you need to initialize (create) your critical section (presumably in your form's constructor or create event)...

InitializeCriticalSection(FLock);

Now you're ready to use it as a lock. When you need to access the object it's protecting, you enter the critical section (locking it)...

EnterCriticalSection(FLock);

Once you have done this, it is locked. No other thread other than the calling thread is able to access this lock. Remember, technically other threads are able to access the object (but would potentially cause a deadlock), so make sure all other threads are also first trying to lock it. That's the goal of this lock.

Once you have done everything you need in this thread, you leave the critical section (unlocking it)...

LeaveCriticalSection(FLock);

Now, it is unlocked and the next thread which may have attempted a lock is now able to completely lock it. That's right, while a critical section is locked, it remembers any other lock attempt, and once the calling thread unlocks it, the next one in the queue automatically acquires this lock. Same concept as a line of people waiting outside a phone booth.

When you're all finished with the object, make sure you dispose of this critical section (presumably in your form's destructor or destroy event)...

DeleteCriticalSection(FLock);

And those are the fundamentals. But we're not done yet.

It's highly advised that you should always use a try..finally block when entering/leaving a critical section. This is to ensure that it gets unlocked, in case of any unhandled exceptions. So, your code should look something like this:

EnterCriticalSection(FLock);
try
  DoSomethingWithTheProtectedObject;
finally
  LeaveCriticalSection(FLock);
end;

Now, the last thing you may wish to do is wrap this up in a nice simple class to do the work for you. This way, you don't have to worry about all the long procedure names whenever you use this lock. This object is called TLocker, which protects an object instance for you. You would need to create an instance of this for every object instance you need to protect. So, instead of declaring something like FMyObject: TMyObject; you would do it like FMyObject: TLocker; and then give the object instance to it upon its creation.

type
  TObjectClass = class of TObject;

  TLocker = class(TObject)
  private
    FLock: TRTLCriticalSection;
    FObject: TObject;
  public
    constructor Create(AObjectClass: TObjectClass); overload;
    constructor Create(AObject: TObject); overload;
    destructor Destroy;
    function Lock: TObject;
    procedure Unlock;
  end;

constructor TLocker.Create(AObjectClass: TObjectClass);
begin
  InitializeCriticalSection(FLock);
  FObject:= AObjectClass.Create;
end;

constructor TLocker.Create(AObject: TObject);
begin
  InitializeCriticalSection(FLock);
  FObject:= AObject;
end;

destructor TLocker.Destroy;
begin
  FObject.Free;
  DeleteCriticalSection(FLock);
end;

function TLocker.Lock: TObject;
begin
  EnterCriticalSection(FLock);
  Result:= FObject; //Note how this is called AFTER the lock is engaged
end;

procedure TLocker.Unlock;
begin
  LeaveCriticalSection(FLock);
end;

The first constructor TLocker.Create(AObjectClass: TObjectClass); expects a class type to be specified (such as TMyObject).

The other constructor TLocker.Create(AObject: TObject); expects an instance of an object already created. Bear in mind that if you do use this constructor, you should no longer refer directly to that object after passing it into this constructor.

In both of these cases, your object which is being protected will be automatically free'd when the TLocker is free'd.

Let's assume the second constructor of the two. Once you've created an instance of this class with your initial actual object reference, never reference to the actual object again. Instead, whenever you need to use that object, pull it out of the TLocker instance by locking it. Using a try..finally block, make sure you also unlock it once you're done, or else the lock is stuck.

So, using the class above, you can access an object like this:

procedure TForm1.FormCreate(Sender: TObject);
var
  O: TMyObject;
begin
  O:= TMyObject.Create;
  FMyObjectLocker:= TLocker.Create(O);
end;

procedure TForm1.FormDestroy(Sender: TObject);
begin
  FMyObjectLocker.Free; //Your original object will be free'd automatically
end;

procedure TForm1.DoSomething;
var
  O: TMyObject;
begin
  O:= TMyObject(FMyObjectLocker.Lock);
  try
    O.DoSomething;

    //etc...

  finally
    FMyObjectLocker.Unlock;
  end;
end;

As you see, I acquire the object at the same time as locking it. This should be the only possible way to access this object. Just make sure you always unlock it when you're done. Notice also how I also don't define the actual protected object in the class - to prevent accidental direct calls to the object.

Answer 1

Your first paragraph says "this is the most commonly used method" without saying what "this" is. When writing a FAQ answer, especially one about something as thorny as multithreading, it's important to be precise from the very start. Maybe "this" refers to something from the question, but I don't know because you didn't include the question in what you presented for review.

You say accessing the same memory from multiple threads leads to deadlock, which is generally false. Accessing the same memory at the same time is a race condition that might be an issue, but also might be harmless. If it leads to deadlock, you probably have buggy hardware.

Furthermore, if you have two threads going back and forth fighting over access, you again don't have deadlock. With deadlock, there's no back-and-forth anymore. If threads are repeatedly going back and forth, that's often an indication of a lock convoy, which is perfectly safe, but doesn't usually provide the desired level of performance.

A critical section is just one way of writing thread-safe code, so it's incorrect to say that creating one begins the way of protecting any object.

Critical section is not a proper noun; there's no need to capitalize it.

Regarding the phone booth, deadlock rarely involves a fight. The CPU doesn't pit two threads against each other and grant memory access to whichever is mightier. It happily grants access to both at the same time. Deadlock would be when someone inside the phone booth calls the home of another person in line for the booth. That person cannot answer his home phone because he's in line outside waiting for the booth, but the caller won't get off the phone until it's answered. This actually illustrates how adding locks can actually lead to deadlock, not prevent it. (When you have no threading protection at all, deadlock is rare.)

You characterize the threads waiting to acquire a critical section as a line of people outside the phone booth. MSDN specifically contradicts that analogy:

There is no guarantee about the order in which waiting threads will acquire ownership of the critical section.

Don't forget to mark TLocker.Destroy with override.

The TLocker constructor that takes a metaclass is of limited utility, especially in the case demonstrated here with TObjectClass (also known as TClass). No matter what class is passed as an argument, the TLocker constructor will always call the zero-argument constructor introduced in the base class, and yet inspecting the ClassType of the created object, it will appear to be of the correct class. For it to call the proper constructor, the Create method of the base class would need to be declared virtual.

I'd forego the metaclass constructor entirely. Leave the task of constructing objects to some other code; your class is called a TLocker, after all, not a TLockerAndObjectFactory.

In TLocker.Lock, you call specific attention to the order of the two instructions. That reveals a misunderstanding on your part because the order of those instructions doesn't make any difference at all. Assigning Result doesn't grant any access to the object before the lock is acquired. Even if it did, that access would be useless because the thread that access was granted to is the current thread, and that thread can't possibly do anything with the object until Lock returns.

There is no apostrophe in freed. It's an ordinary English word.

You mention how you "don't define the actual protected object in the class." That doesn't make any sense. I think what you wanted to convey was that you don't expose the protected object except through the Lock method. You want to point out that the FObject field is private intentionally.

Ultimately, it's hard to judge how good a FAQ answer this is because the question it's meant to answer is absent. There's systemic confusion over what deadlock is and how it occurs, so I hope the question didn't ask about deadlock. You might just want to skip the introductory motivation and go straight to presenting the locking code.

Answer 2

As others have stated, a deadlock occurs when 2 (or more threads) vie for the same resource (object/handle/int/etc) and a thread does not release the lock appropriately:

// thread 1 and thread 2 call the following:
void threadfn() {
     EnterCriticalSection(&mtx);
     // do some work
}

A dead lock condition will occur in the above code because a thread 'locked' the mutex (critical section) then never unlocked it, so the other thread who is blocking on EnterCriticalSection will stay blocked until a call to LeaveCriticalSection is made.

A quick note on CRITICAL_SECTION 'objects' (handles): they are recursive/re-entrant by the same thread. In other words, given the above code, if thread 1 calls threadfn 10 times, then 10 calls to EnterCriticalSection would be made and thread 1 won't lock, but the same thread (thread 1) would have to call LeaveCriticalSection the same number of times (10 in our example) before any other thread would be able to 'grab' the lock.

Using the CreateSeamphore function, you can create a binary semaphore (a sempaphore with only 1 active lock); binary semaphores on the Windows platform are non re-entrant (non-recursive).

There's also the TryEnterCriticalSection function which returns TRUE if the 'lock' operation succeeds.

Another note: you stated that "It's highly advised that you should always use a try..finally block when entering/leaving a critical section", per the MSDN documentation on EnterCriticalSection, it's not a good idea to try...catch a critical section error (and I agree) for numerous reasons; instead it's a better idea to debug WHY it threw the error which happens on a timeout of the critical section which indicates a genuine problem occurred that needs to be addressed in code and not just 'caught away' as it could indicate a deadlock.

Last note: you talk in your FAQ about protecting 'objects' from a multi-threaded environment and present this via a Delphi/MS context using a TForm as an example, this is fine but if this FAQ is supposed to be on multi-threading (from this FAQ it looks specifically to be about the 'mutex' idiom WRT WinAPI) then try focusing specifically on that (smaller code snippets losing the TForms, just simple examples of multiple threads accessing the same 'object' .. an integer is fine for teaching purposes). You also need to make a note about how CRITICAL_SECTION's don't like to be copied (i.e. if a copy constructor were to be introduced and you 'copied' the critical section from one object to the other), that behavior is undefined (as it should be).

I hope that's helpful