StateClient object instances are stored in thread local storage. Each thread can only have one instance of this object. There is however one shared list inside of TLSOjbect2 that holds references to all StateClient objects created in a given process. The instances of
StateClient can be destroyed in two ways. 1. Something calls dfb_gfx_cleanup(). 2. Thread terminates. In the first case the function is called once when the process using directfb shuts down. This happens ony once during application life. The function iterates over all StateClient objects present in the TLSObject2::list and removes each one. The second case libc will call the callback function passed to direct_tls_register() when the thread which registered the tls object terminates. The problem is that while dfb_gfx_cleanup() does remove the deleted objects from the TLSObject2::list, the default callback function passed to direct_tls_register (TLSObject2::destructor) does not. As a result the following situation will lead to a double-free memory corruption. 1. The application starts and initializes directfb. 2. At least one extra thread is started, this thread uses some dfb APIs which internally create new StateClient object. 3. The extra thread terminates thus invokes TLSObject2::destructor. 4. The application exits, dfb_gfx_cleanup() is called, StateClient for the extra thread is deleted for the second time. The problem is nicely illustrated by valgrind. In this case dfbterm is used as the application triggering the problem. ==336== Invalid read of size 4 ==336== at 0x4EA4120: CoreGraphicsStateClient_Deinit (CoreGraphicsStateClient.cpp:387) ==336== by 0x4F7165F: StateClient::~StateClient() (util.cpp:78) ==336== by 0x4F71A07: Direct::TLSObject2<StateClient, StateClient, StateClient>::DeleteAll() (TLSObject.h:175) ==336== by 0x4EE3130: dfb_core_shutdown (core.c:1734) ==336== by 0x4EE36F3: dfb_core_arena_shutdown (core.c:2020) ==336== by 0x4EE418C: dfb_core_destroy (core.c:472) ==336== by 0x4E969A2: IDirectFB_Destruct (idirectfb.c:307) ==336== by 0x402813: main (term.c:523) ==336== Address 0x8b37730 is 1,072 bytes inside a block of size 1,136 free'd ==336== at 0x4C28D2C: operator delete(void*) (vg_replace_malloc.c:482) ==337== by 0x5233F81: __nptl_deallocate_tsd (pthread_create.c:158) ==337== by 0x52341E3: start_thread (pthread_create.c:325) ==336== by 0x5B7A0DC: clone (clone.S:111) This bug also triggers the following assertion: Assertion [(client)->magic == D_MAGIC("CoreGraphicsStateClient")] failed *** [CoreGraphicsStateClient.cpp:387 in CoreGraphicsStateClient_Deinit()] This patch is also applicable for directfb-1.7 branch. Signed-off-by: Peter Tworek <tworaz...@gmail.com> --- src/gfx/util.cpp | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/gfx/util.cpp b/src/gfx/util.cpp index a432dcd..5d3cc73 100644 --- a/src/gfx/util.cpp +++ b/src/gfx/util.cpp @@ -84,13 +84,24 @@ public: static void destroy( void *ctx, StateClient *client ) { - delete client; + _tls.Delete(); } -}; + static StateClient* Get() + { + return _tls.Get(); + } -static Direct::TLSObject2<StateClient> state_client_tls; + static void DeleteAll() + { + _tls.DeleteAll(); + } + +private: + static Direct::TLSObject2<StateClient> _tls; +}; +static Direct::TLSObject2<StateClient> StateClient::_tls; extern "C" { @@ -98,13 +109,13 @@ extern "C" { void dfb_gfx_init_tls() { - state_client_tls.Get(); + StateClient::Get(); } void dfb_gfx_cleanup() { - state_client_tls.DeleteAll(); + StateClient::DeleteAll(); } void @@ -140,7 +151,7 @@ dfb_gfx_copy_stereo( CoreSurface *source, { DFBRectangle sourcerect = { 0, 0, source->config.size.w, source->config.size.h }; - StateClient *client = state_client_tls.Get(); + StateClient *client = StateClient::Get(); D_FLAGS_SET( client->state.modified, SMF_CLIP | SMF_SOURCE | SMF_DESTINATION | SMF_FROM | SMF_TO ); @@ -180,7 +191,7 @@ dfb_gfx_clear( CoreSurface *surface, CoreSurfaceBufferRole role ) { DFBRectangle rect = { 0, 0, surface->config.size.w, surface->config.size.h }; - StateClient *client = state_client_tls.Get(); + StateClient *client = StateClient::Get(); D_FLAGS_SET( client->state.modified, SMF_CLIP | SMF_COLOR | SMF_DESTINATION | SMF_TO ); @@ -237,7 +248,7 @@ void dfb_gfx_stretch_stereo( CoreSurface *source, return; } - StateClient *client = state_client_tls.Get(); + StateClient *client = StateClient::Get(); D_FLAGS_SET( client->state.modified, SMF_CLIP | SMF_SOURCE | SMF_DESTINATION | SMF_FROM | SMF_TO ); @@ -309,7 +320,7 @@ dfb_gfx_copy_regions_stereo( CoreSurface *source, } if (n > 0) { - StateClient *client = state_client_tls.Get(); + StateClient *client = StateClient::Get(); D_FLAGS_SET( client->state.modified, SMF_CLIP | SMF_SOURCE | SMF_DESTINATION | SMF_FROM | SMF_TO ); @@ -351,7 +362,7 @@ dfb_gfx_copy_regions_client( CoreSurface *source, DFBRectangle rect = { 0, 0, source->config.size.w, source->config.size.h }; DFBRectangle rects[num]; DFBPoint points[num]; - CoreGraphicsStateClient *client = _client ? _client : &state_client_tls.Get()->client; + CoreGraphicsStateClient *client = _client ? _client : &StateClient::Get()->client; CardState *state = client->state; CardState backup; @@ -422,7 +433,7 @@ back_to_front_copy( CoreSurface *surface, { DFBRectangle rect; DFBPoint point; - StateClient *client = state_client_tls.Get(); + StateClient *client = StateClient::Get(); if (region) { -- 1.8.1.5 _______________________________________________ directfb-dev mailing list directfb-dev@directfb.org http://mail.directfb.org/cgi-bin/mailman/listinfo/directfb-dev