From 678a2c3042b61a87280eef48c52c47cc83bd2d8d Mon Sep 17 00:00:00 2001 From: Rajeev Ranjan Date: Sat, 18 May 2013 16:27:35 +0530 Subject: [PATCH] Memory Leak Fixes In Remix Change-Id: I49c955c81e62b3d08692d29bb6e5f56be30d3bf4 --- src/ctxdata/cd_list.c | 22 ++++++++++++++++++++++ src/libremix/remix_base.c | 12 ++++++++++-- src/libremix/remix_context.c | 13 ++++++++----- src/libremix/remix_deck.c | 4 +++- src/libremix/remix_layer.c | 5 ++++- src/libremix/remix_track.c | 9 ++++++++- 6 files changed, 55 insertions(+), 10 deletions(-) diff --git a/src/ctxdata/cd_list.c b/src/ctxdata/cd_list.c index 2bd38aa..b142b6e 100644 --- a/src/ctxdata/cd_list.c +++ b/src/ctxdata/cd_list.c @@ -379,6 +379,10 @@ cd_list_remove (void * ctx, CDList * list, CDScalarType type, CDScalar data) if (l->prev) l->prev->next = l->next; if (l->next) l->next->prev = l->prev; + //free the element here to avoid memory leak, otherwise reference is lost + //and a dangling pointer. + cd_free (l); + if (l == list) return list->next; else return list; } @@ -443,6 +447,24 @@ cd_list_destroy_with (void * ctx, CDList * list, CDDestroyFunc destroy) } /* + * cd_list_call_destroy (ctx, list, destroy) + * + * Step through list 'list', calling AxDestroyFunc destroy function on each. + */ +CDList * +cd_list_call_destroy (void * ctx, CDList * list, CDDestroyFunc destroy) +{ + CDList * l, * ln; + + for (l = list; l; l = ln) { + ln = l->next; + destroy (ctx, l->data.s_pointer); + } + + return NULL; +} + +/* * cd_list_free_all (ctx, list) * * Step through list 'list', freeing each item using cd_free(), and diff --git a/src/libremix/remix_base.c b/src/libremix/remix_base.c index bb51fee..7bd74dd 100644 --- a/src/libremix/remix_base.c +++ b/src/libremix/remix_base.c @@ -194,7 +194,8 @@ remix_base_new_subclass (RemixEnv * env, size_t size) { RemixBase * base = remix_malloc (size); _remix_context_copy (env, &base->context_limit); - _remix_register_base (env, base); + //Don't register/unregister base unncessarily + //_remix_register_base (env, base); return base; } @@ -370,12 +371,19 @@ remix_destroy (RemixEnv * env, RemixBase * base) return -1; } - _remix_unregister_base (env, base); + //Don't register/unregister base unncessarily + //_remix_unregister_base (env, base); if (!base->methods || !base->methods->destroy) { remix_set_error (env, REMIX_ERROR_INVALID); return -1; } + + if (base && base->context_limit.channels) { + //free the cloned copy of channels to avoid memory leak. + cd_set_free(env, base->context_limit.channels); + } + return _remix_destroy (env, base); } diff --git a/src/libremix/remix_context.c b/src/libremix/remix_context.c index 02cb97e..1a04ad4 100644 --- a/src/libremix/remix_context.c +++ b/src/libremix/remix_context.c @@ -134,7 +134,8 @@ remix_init (void) world->refcount = 0; world->plugins = cd_list_new (ctx); - world->bases = cd_list_new (ctx); + //No need to record bases, as individual destroy will take care. Commenting to Fix Memory leak. + //world->bases = cd_list_new (ctx); world->purging = FALSE; ctx->mixlength = REMIX_DEFAULT_MIXLENGTH; @@ -273,8 +274,9 @@ _remix_unregister_plugin (RemixEnv * env, RemixPlugin * plugin) RemixEnv * _remix_register_base (RemixEnv * env, RemixBase * base) { - RemixWorld * world = env->world; - world->bases = cd_list_append (env, world->bases, CD_POINTER(base)); + //As world->bases is not initialized and used, so should not add it to env + //RemixWorld * world = env->world; + //world->bases = cd_list_append (env, world->bases, CD_POINTER(base)); return env; } @@ -284,8 +286,9 @@ _remix_unregister_base (RemixEnv * env, RemixBase * base) RemixWorld * world = env->world; if (world->purging) return env; - world->bases = cd_list_remove (env, world->bases, CD_TYPE_POINTER, - CD_POINTER(base)); + //As world->bases is not initialized and used, so should not remove it from env + //world->bases = cd_list_remove (env, world->bases, CD_TYPE_POINTER, + // CD_POINTER(base)); return env; } diff --git a/src/libremix/remix_deck.c b/src/libremix/remix_deck.c index c0206f7..ed38934 100644 --- a/src/libremix/remix_deck.c +++ b/src/libremix/remix_deck.c @@ -92,7 +92,9 @@ static int remix_deck_destroy (RemixEnv * env, RemixBase * base) { RemixDeck * deck = (RemixDeck *)base; - remix_destroy_list (env, deck->tracks); + //Just call destructor but dont delete item which is removed already. + //remix_destroy_list (env, deck->tracks); + cd_list_call_destroy(env, deck->tracks, (CDDestroyFunc)remix_destroy); remix_destroy (env, (RemixBase *)deck->_mixstream); remix_free (deck); return 0; diff --git a/src/libremix/remix_layer.c b/src/libremix/remix_layer.c index 158fe09..9eb61f3 100644 --- a/src/libremix/remix_layer.c +++ b/src/libremix/remix_layer.c @@ -114,9 +114,12 @@ static int remix_layer_destroy (RemixEnv * env, RemixBase * base) { RemixLayer * layer = (RemixLayer *)base; + if (layer->track) _remix_track_remove_layer (env, layer->track, layer); - remix_destroy_list (env, layer->sounds); + //Just call destructor but dont delete item which is removed already. + //remix_destroy_list (env, layer->sounds); + cd_list_call_destroy(env, layer->sounds, (CDDestroyFunc)remix_destroy); remix_free (layer); return 0; } diff --git a/src/libremix/remix_track.c b/src/libremix/remix_track.c index 0cf2eb2..433d27a 100644 --- a/src/libremix/remix_track.c +++ b/src/libremix/remix_track.c @@ -94,7 +94,14 @@ static int remix_track_destroy (RemixEnv * env, RemixBase * base) { RemixTrack * track = (RemixTrack *)base; - remix_destroy_list (env, track->layers); + //Just call destructor but dont delete item which is removed already. + //remix_destroy_list (env, track->layers); + cd_list_call_destroy(env, track->layers, (CDDestroyFunc)remix_destroy); + + if (track->_mixstream_a != RemixNone) + remix_destroy (env, (RemixBase *)track->_mixstream_a); + if (track->_mixstream_b != RemixNone) + remix_destroy (env, (RemixBase *)track->_mixstream_b); remix_free (track); return 0; } -- 2.7.4