Delphi threading: CriticalSection not being "Release'd" when using Synchronize inside its method

Question

In my project I have a Thread which might be modiefied by the thread itself, other thread or VCL (main app). Thus I'm using TCriticalSection.Acquire / Release for every data access.

Under normal circumstances, the below code works as expected: enters Acquire, Synchronizes with DoCallback, then releases the lock. However, if any of other contexts Acquires the lock at the time it was already locked, the execution of below code stops at Synchronize - and, this time, it DOES NOT enter the DoCallback method.

Should I skip Synchronize method (even though the Synchronize'd code calls VCL) and rely on the CriticalSection itself? What is the reason of this behavior?

code of the main thread:

  fData:= nil;
  try
    fSingleRequest.Acquire;      
    if fItem <> nil then
      begin
        fData:= fItem.Request;
        SubmitRequest();
        fCallbackData:= fItem.fExtraData;
        fCallback:= fItem.fCallback;
        Synchronize(DoCallback); // <-- this line is called
      end;
  finally
    fSingleRequest.Release;      // <-- this isn't under the specific situation
  end;

Answer 1

If your so-called "main" thread acquires the critical section, and then the VCL thread (which is what we would usually call the "main" thread) attempts to acquire it, then the VCL thread will block until the critical section is released. Then your "main" thread calls Synchronize, which runs the given function in the context of the VCL thread. Since the VCL thread is blocked waiting on the critical section, it's not processing messages, so it can't notice that there's a synchronized method to call. Thus, deadlock.

Don't hold the lock across intra-thread calls. Release the lock in your "main" thread before you call Synchronize, and then re-acquire it afterward, if you still need it. If the data being used in the synchronized method needs continued protection from simultaneous access, then I think you should find a way to copy the data into a separate, non-shared object. Have the synchronized method use that object instead of the shared one, and then destroy it afterward.

Your comment indicates that you can call the callback function without Synchronize and everything appears to work. In that case, the synchronized method isn't being called in the same thread context as before. It's being called directly by your "main" thread instead of by the VCL thread. That obviously relieves the lock contention, but whether it's really safe depends on what the callback function does.