Watch, Follow, &
Connect with Us

Please visit our new home
community.embarcadero.com.


Welcome, Guest
Guest Settings
Help

Thread: Memory leak using _beginthread/_endthread in 64-bit C++ application


This question is not answered. Helpful answers available: 2. Correct answers available: 1.


Permlink Replies: 11 - Last Post: Aug 30, 2017 3:03 PM Last Post By: Remy Lebeau (Te... Threads: [ Previous | Next ]
Clayton Arends


Posts: 25
Registered: 7/19/01
Memory leak using _beginthread/_endthread in 64-bit C++ application  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 29, 2017 6:52 PM
I'm encountering a memory leak involving multiple threads in a C++Builder 64-bit application. The 32-bit counterpart does not have the leak. My actual application uses TThread but I am able to reproduce the leak using _beginthread and _endthread. I have reproduced the leak in both a VCL Forms and a Console Application.

Here is the simplest code demonstrating the leak in a VCL Forms Application:
void ThreadCode(void*)
{
  _endthread();
}
 
void __fastcall TForm1::Button1Click(TObject* Sender)
{
  for (int i = 0; i < 500000; i++)
    _beginthread(ThreadCode, 0, NULL);
}


This code leaks approximately 9,300 bytes per thread. Creating 500000 threads my application consumes 4.5gb. The leak in a non-VCL console application isn't as substantial at 120mb. I was about to submit a bug report but figured I would ask others if they've encountered this problem, reproduce the bug, or maybe provide some insight into project settings that should be adjusted.

I can produce the leak in Berlin (10.1 Update 2) and Tokyo (10.2 Release 1).
Mike Versteeg

Posts: 118
Registered: 9/16/07
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 2:46 AM   in response to: Clayton Arends in response to: Clayton Arends
I use TThread intensively and never noticed this. But I do not repeatedly create/destroy threads, and certainly not 500000 :). C++ Builder Pro 10.2.1.
Clayton Arends


Posts: 25
Registered: 7/19/01
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 7:29 AM   in response to: Mike Versteeg in response to: Mike Versteeg
Hi Mike,

Thank you for your reply. By chance did you attempt to reproduce the leak with my code?

Using TThread alone isn't enough to cause the leak. The RTL global thread data object must be used. Perhaps I should have posted this example as well. Here is code to reproduce using TThread. Note, this code will leak in 32-bit C++ applications as well. I submitted this as a bug report using Quality Central years ago but will need to resubmit to the new Quality portal.

class TLeakThread : public TThread
{
  protected:
    virtual void __fastcall Execute()
    {
      randomize(); // initializes per-thread data
    }
 
  public:
    TLeakThread() : TThread(false)
    {
      FreeOnTerminate = true;
    }
};
 
void __fastcall TForm1::Button1Click(TObject* Sender)
{
  for (int i = 0; i < 500000; i++)
  {
    new TLeakThread();
  }
}


Can you take a moment to attempt to reproduce the leak using this code?

Clayton
Mike Versteeg

Posts: 118
Registered: 9/16/07
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 8:08 AM   in response to: Clayton Arends in response to: Clayton Arends
Not in my office right now (and this forum is not mobile friendly!), but is randomize thread safe? Why not exclude that? Also I think I read somewhere about issues with freeonterminate, try manually delete.
Clayton Arends


Posts: 25
Registered: 7/19/01
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 8:56 AM   in response to: Mike Versteeg in response to: Mike Versteeg
Mike,

Yes, randomize() is thread safe. It can't be excluded because it's important in understanding this particular leak in that something needs to call _thread_data() (see $(BDS)\source\cpprtl\Source\threads\thrddata.c). However, instead of randomize() one can use any function that initializes the C RTL per-thread data to introduce the same problem. I can write an application that doesn't expose this leak using a TThread descendant by carefully making sure that no C RTL function is called that eventually calls _thread_data(). However, large applications that make use of multiple third-party libraries can't always have this assurance. There is a workaround for fixing this particular leak using _adoptthread but that only completely works for 32-bit. 64-bit continues to leak.

FreeOnTerminate is safe. But, to demonstrate that this isn't the problem here is the code sans FreeOnTerminate.

class TLeakThread : public TThread
{
  protected:
    virtual void __fastcall Execute()
    {
      randomize(); // or use _tcstok(), __errno(), etc
    }
 
  public:
    TLeakThread() : TThread(false)
    {
    }
};
 
void CreateThreads()
{
  for (int i = 0; i < 500000; i++)
  {
    TLeakThread* thread = new TLeakThread();
    thread->WaitFor();
    delete thread;
  }
}


Thank you for your time. I'm curious to see your results if you test any of the code that I've posted.

Clayton
Remy Lebeau (Te...


Posts: 8,940
Registered: 12/23/01
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application [Edit]  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 10:12 AM   in response to: Clayton Arends in response to: Clayton Arends
Clayton Arends wrote:

Using TThread alone isn't enough to cause the leak. The RTL global
thread data object must be used. Perhaps I should have posted this
example as well.

Makes sense. randomize() uses TLS data, and that data is not being
freed when each thread terminates, hense the leaking.

That is not really TThread's fault. It has no knowledge of the C RTL,
so it doesn't know TLS data is being allocated that needs to be freed.
Remember, TThread is implemented in Delphi, not C++. It uses
CreateThread() directly instead of _beginthread().

As for _beginthread()/_endthread(), leaking TLS data is a known issue
for it (and that is not specific to Borland/Embarcadero compilers,
either). You have to use _beginthreadex() and _endthreadex() instead
when dealing with TLS data.

http://blog.aaronballman.com/2011/09/threading-on-windows/

--
Remy Lebeau (TeamB)
Clayton Arends


Posts: 25
Registered: 7/19/01
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application [Edit]  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 11:54 AM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Hi Remy,

Thank you for your response.

Makes sense. randomize() uses TLS data, and that data is not being
freed when each thread terminates, hense the leaking.

That is not really TThread's fault. It has no knowledge of the C RTL,
so it doesn't know TLS data is being allocated that needs to be freed.

Yes, and using _adopt_thread() can fix the leak in a 32-bit application. It's not fixing it in the 64-bit application. I'll post this code in a different message.

As for _beginthread()/_endthread(), leaking TLS data is a known issue
for it (and that is not specific to Borland/Embarcadero compilers,
either). You have to use _beginthreadex() and _endthreadex() instead
when dealing with TLS data.

I've changed my code to use the ex functions (using MSDN examples) with the same result. Here is the new code:

unsigned int __stdcall ThreadCode(void*)
{
  _endthreadex(0);
  return 0;
}
 
void __fastcall TForm1::Button1Click(TObject* Sender)
{
  for (int i = 0; i < 500000; i++)
  {
    unsigned int threadId;
    HANDLE hThread = (HANDLE) _beginthreadex(NULL, 0, ThreadCode, NULL, 0, &threadId);
    WaitForSingleObject(hThread, INFINITE);
    CloseHandle(hThread);
  }
}


Clayton
Clayton Arends


Posts: 25
Registered: 7/19/01
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application [Edit]  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 12:18 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
As promised, here is the code I use to patch TThread when it uses TLS data.

#include <process.h>
 
typedef void __fastcall (__closure *TEmptyMethod)();
struct Adoption
{
  _PTHREAD_ADOPTION_DATA tad;
 
  static void adopted_thread(void* handler)
  {
    try
    {
      TEmptyMethod ec = *static_cast <TEmptyMethod*> (handler);
      ec();
    }
    catch (...)
    {
    }
  }
 
  Adoption(TEmptyMethod Handler)
  {
    // Let the RTL adopt the thread to manage global memory
    tad = _adopt_thread(adopted_thread, &Handler, FALSE);
  }
 
  ~Adoption()
  {
    _unadopt_thread(tad);
  }
};
 
class TLeakThread : public TThread
{
  private:
    void __fastcall NewExecute()
    {
      randomize();
    }
 
  protected:
    virtual void __fastcall Execute()
    {
      Adoption adoption(&NewExecute);
    }
 
  public:
    __fastcall TLeakThread() : TThread(false)
    {
      FreeOnTerminate = true;
    }
};
 
void __fastcall TForm1::Button1Click(TObject* Sender)
{
  for (int i = 0; i < 500000; i++)
  {
    new TLeakThread();
  }
}


A 32-bit application will not leak. A 64-bit application will leak. Note, it leaks slightly less (~9380 / thread) when using adopt than if not using (~9550 / thread).

Clayton
Clayton Arends


Posts: 25
Registered: 7/19/01
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application [Edit]  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 12:25 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Remy Lebeau (TeamB) wrote:
You have to use _beginthreadex() and _endthreadex() instead
when dealing with TLS data.

http://blog.aaronballman.com/2011/09/threading-on-windows/

Note, with modern source this is probably a moot point. By looking at $(BDS)\source\cpprtl\Source\threads\thread.c all roads lead to __beginthreadNT().

Clayton
Remy Lebeau (Te...


Posts: 8,940
Registered: 12/23/01
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application [Edit]  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 2:06 PM   in response to: Clayton Arends in response to: Clayton Arends
Clayton Arends wrote:

Note, with modern source this is probably a moot point. By looking
at $(BDS)\source\cpprtl\Source\threads\thread.c all roads lead to
__beginthreadNT().

Maybe, maybe not.

On Windows, TThread doesn't use __beginthread/ex() at all. It calls
System::BeginThread(), which calls System::SystemThreadFuncProc if
assigned, before then calling the Win32 API CreateThread() directly.

When TThread ends, it calls System:::EndThread(), which calls
System::SystemThreadEndProc if assigned, before then calling the Win32
API ExitThread() directly.

The C++ RTL is supposed to hook up SystemThreadFuncProc and
SystemThreadEndProc to manage C++ TLS data in TThread (see
CppThreadFuncProc() and CppThreadEndProc() in
$(BDS)\source\cpprtl\source\vcl\crtl_thd.c). CppThreadEndProc() calls
_endthreadex().

However, in __CRTL_VCL_Init() (see
$(BDS)\source\cpprtl\source\vcl\crtlvcl.cpp), those hookups are
commented out.

When I create a C++ VCL project and createa new TThread object, those
callbacks are not NULL, but I can't see what they are actually assigned
to (the debugger won't let me step into them).

So, TThread MAY ORMAY NOT manage TLS data in C++.

--
Remy Lebeau (TeamB)
Clayton Arends


Posts: 25
Registered: 7/19/01
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application [Edit]  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 2:50 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Remy Lebeau (TeamB) wrote:
Clayton Arends wrote:

Note, with modern source this is probably a moot point. By looking
at $(BDS)\source\cpprtl\Source\threads\thread.c all roads lead to
__beginthreadNT().

Maybe, maybe not.

I was referring specifically to the point that _beginthreadex()/_endthreadex() should be used instead of _beginthread()/_endthread. I meant that both _beginthread() and _beginthreadex() end up at _beginthreadNT(). I wasn't implying that TThread invokes _beginthreadNT() but I can see how that may have been unclear by my glib use of the phrase "all roads lead to".

The C++ RTL is supposed to hook up SystemThreadFuncProc and
SystemThreadEndProc to manage C++ TLS data in TThread (see
CppThreadFuncProc() and CppThreadEndProc() in
$(BDS)\source\cpprtl\source\vcl\crtl_thd.c). CppThreadEndProc() calls
_endthreadex().

However, in __CRTL_VCL_Init() (see
$(BDS)\source\cpprtl\source\vcl\crtlvcl.cpp), those hookups are
commented out.

Interesting. That code is new to me since the last time I went digging. The code in crtl_thd.cpp uses _adopt_thread() to wrap the TLS allocation. The code is a little more involved than the solution from my other post.

Thank you for pointing this out. By chance have you had a moment to attempt either of the memory leaks in 64-bit?

Clayton
Remy Lebeau (Te...


Posts: 8,940
Registered: 12/23/01
Re: Memory leak using _beginthread/_endthread in 64-bit C++ application [Edit]  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Aug 30, 2017 3:03 PM   in response to: Clayton Arends in response to: Clayton Arends
Clayton Arends wrote:

By chance have you had a moment to attempt either of the memory leaks
in 64-bit?

No, I haven't.

--
Remy Lebeau (TeamB)

Legend
Helpful Answer (5 pts)
Correct Answer (10 pts)

Server Response from: ETNAJIVE02