Possible stack corruption inside try __finally block

Question

I am having problems with stack corruption in a new module I am working on which is part of a large legacy project. My code is written in C++ using Borland C++Builder 5.0.

I have tracked the problem to the following function:

// Note: Class TMarshalServerClientThread has the following objects defined
// CRITICAL_SECTION                 FCriticalSection;
// std::vector<TMarshalTagInfo*>    FTagChangeQueue;

void __fastcall TMarshalServerClientThread::SendChangeNotifications()
{
   EnterCriticalSection(FCriticalSection);

   try {
      if (FTagChangeQueue.size() == 0) {         
         return;
      }

      // Process items in change queue

      FTagChangeQueue.clear();

   } __finally {
      LeaveCriticalSection(FCriticalSection);
   }
}

This function is called in the context of a worker thread (which descends from TThread). A different thread populates the change queue with data as it becomes available. The change queue is protected by a critical section object.

When the code is run, I sporadically get access violations when attempting to leave the critical section. From what I can tell, sometimes when the __finally section is entered, the stack is corrupted. The class instance on the heap is fine, but the pointers to the class (such as the "this" pointer) appear to be invalid.

If I remove the call to return if the change queue is empty, the problem goes away. Additionally, the code to process the items in the queue is not the source of the problem, as I can comment it out and the problem remains.

So my question is are there known issues when using __finally in C++Builder 5? Is it wrong to call return from within a try __finally block? If so, why?

Please note that I realize that there are different/better ways to do what I am doing, and I am refactoring as such. However, I fail to see why this codes should be causing stack corruption.

Answer 1

As @duDE pointed, you should use pair of __try, __finally instead of intermixing C++ try, and Borland extension __finally.

Answer 2

I know it is a long time after the original question was posted, but as a warning to others, I can vouch for the symptom that Jonathan Wiens is reporting. I experienced it with Builder XE4. It does not happen frequently, but it seems that Borland/Embarcadero's implementation of try / finally blocks in a multi-threaded process very occasionally corrupts the stack. I was also using critical sections, although that may be coincidental.

I was able to resolve my problem by discarding the try / finally. I was fortunate that I was only deleting class instances in the finally block, so I was able to replace the try / finally with scope braces, using std::auto_ptr fields to delete the objects in question.