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

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 p

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.

StackExchange Code Review Q#33427, answer score: 3