[Ace-users] Re: [ace-bugs] ACE_Service_Config TSS usage: Improper TSS usage/memory management

[Andrew Keleher]

Hi Patrick,
 
I found this same issue back in April and it is being tracked in ACE
bugzilla as bug 2915. Aleksandar Vukajlovic came up with a simple test
program that allows the problem to be reproduced (see below). Aleksandar
also came up with a proposed solution, however the assumption was that
the keys were leaked, and not due to pointer indirection. ACE has yet to
validate Aleksandar's proposed patch.

I have tried to debug the ACE code myself and had no luck...
Aleksandar's analysis is as follows:
1.First not released key is used internaly in ACE_TSS_Cleanup called 
"in_use_" key.
       Instead of empty destructor
ACE_TSS_Cleanup::~ACE_TSS_Cleanup ()
{
}

there must be
ACE_TSS_Cleanup::~ACE_TSS_Cleanup (void)
{
 if (this->in_use_ != ACE_OS::NULL_key)
 {
  this->free_key (this->in_use_);
 }
}


2. Second not released key is ACE_Log_Msg key. According to comments in

function:
void ACE_Log_Msg::close (void) somebody expected that ACE_TSS_Cleanup
will 
remove key of ACE_Log_Msg on closing,
but that is not the case (because ACE_TSS_Cleanup singleton maintains
per 
thread tls object destruction not a global one )

We need to call  ACE_Thread::keyfree (*(log_msg_tss_key ())); after
if (key_created_ == 1)
{
.....

ACE_Thread::keyfree (*(log_msg_tss_key ())); //missing line
} 



 
Sample code to reproduce issue:
 
#include "ace/ACE.h"
#include "ace/OS_NS_Thread.h"
#include "ace/streams.h"
 
#define N 1000
 
void doTest ()
{
 
 for (int i = 0; i < ACE_DEFAULT_THREAD_KEYS + N; ++i)
 {
  try
  {
   ACE::init ();
  }
  catch (...)
  {
   std::cout << "Unknown exception in ACE::init () - iteration " << i
<< std::endl;
 
   //just to be sure
   DWORD out_of_key = ::TlsAlloc ();
   if (out_of_key == TLS_OUT_OF_INDEXES)
    std::cout << "ACE::init out of TLS " << std::endl;
 
   return;
  }
  
  DWORD next_available = ::TlsAlloc ();
  ::TlsFree (next_available);
  
  std::cout << "Last available TLS key after init = " << next_available
<< " (iteration = "<< i << ")" << std::endl;
 
  try
  {
   ACE::fini ();
  }
  catch (...)
  {
   std::cout << "Unknown exception in ACE::fini () - iteration " << i
<< std::endl;
   return;
  }
 }
}
 
int main (int argc, char* argv[])
{
 doTest ();
 return 0;
}
 

Cheers,
Andrew Keleher

>>> "Bennett, Patrick" <Patrick.Bennett at inin.com> 6/22/2007 3:55 PM
>>>
ACE VERSION: 5.5.7

HOST MACHINE and OPERATING SYSTEM: Windows XP/Vista

COMPILER NAME AND VERSION (AND PATCHLEVEL): VC++ 7.1 SP1

THE $ACE_ROOT/ace/config.h FILE:

#define ACE_DEFINES_DEFAULT_WIN32_SECURITY_ATTRIBUTES
#define ACE_DEFAULT_THREAD_KEYS 128

#include "ace/config-win32.h"

AREA/CLASS/EXAMPLE AFFECTED: ACE_Service_Config and its use of TSS for

pointers to itself (which ACE_TSS can then destroy erroneously) 


DOES THE PROBLEM AFFECT:
        COMPILATION? No
        LINKING? No
        EXECUTION? Yes

SYNOPSIS: When ACE is used within a DLL dynamically loaded into another

process that doesn't use ACE, the use of TLS in ACE_Service_Config
causes 
the heap to be corrupted the heap during thread-detach cleanup.  

DESCRIPTION: ACE_Service_Config uses ACE_TSS to store pointers to
itself

yet ACE_TSS is designed to own all objects assigned to it, deleting
them

as threads are destroyed (via the ACE_OS::cleanup_tss (0); call in 
DllMain's DLL_THREAD_DETACH in OS_NS_stdio.cpp).  

Since ACE_Service_Config was actually created as an aggregate member 
within ACE_Singleton ('new' was called on 
ACE_Singleton<ACE_Service_Config> not ACE_Service_Config!), having the

cleanup method in ACE_TSS call delete on the pointer assigned to it
causes 
it to try to delete the ACE_Service_Config instance pointer even
though
it 
wasn't what was allocated from the heap!  With VC++ at least, this
means

the heap is corrupted as a pointer incorrectly offset by 4 bytes is
passed 
to delete (it's passing the pointer to the aggregated member instead of

the containing class that was actually allocated off the heap).  
ACE_Service_Config has a hack in its destructor to set the 
tss_.ts_object(0) value to 0 so that ACE_TSS won't clean it up, but
this

assumes that the ACE_Service_Config created via a static variable at 
startup was actually created on the main thread!  
Worse yet, it assumes the destructor (object manager cleanup) is
called
on the same thread that called the constructor.  But then again, that
all 
pales compared to deleting an aggregated instance anyway...

For normal console applications, the destructor will be called
before the thread it was created within actually goes away so the 
destructor hack resetting the TSS object will work.  However,
if ACE is used by a DLL loaded dynamically by another process, the 
ACE_Service_Config global isn't necessarily destroyed before the
thread
it 
was created on goes away, so the ts_object isn't reset and 
ACE_TSS::cleanup ends up deleting the 'this' pointer that 
ACE_Service_Config's constructor set to itself.

Unfortunately, this took way too long to track down.  :(


    REPEAT BY:

This was 100% repeatable with a test program, but its not something
that

can easily be posted.  It was basically an Internet Explorer add-in
loaded 
dynamically by IE, and the crash would come as IE terminated the thread

ACE was loaded on.

    SAMPLE FIX/WORKAROUND:

The TSS usage within ACE_Service_Config is completely broken.
Not only does it assume that the thread which created the 
ACE_Service_Config is the same one that will later access it (if not, 
the tss info will be 0), it places pointers to its *aggregated* self 
within a class that assumes ownership and will delete them!  

Perhaps a wrapper class should be allocated/stored in ACE_TSS instead
that 
simply contains a pointer to an ACE_Service_Gestalt and whose
destructor

does nothing. The current() method in ACE_Service_Config should 
also be changed to return the 'global' singleton instance if the 
tss_ object is 0.


Thanks,
Patrick Bennett

_______________________________________________
ace-bugs mailing list
ace-bugs at mail.cse.wustl.edu 
http://mail.cse.wustl.edu/mailman/listinfo/ace-bugs