script: Fix the memory management

Currently, the memory management in ClutterScript is overly complicated.
The basic design tenet should be:

  - ClutterScript owns a reference on every object it creates

This allows the Script instance to reliably handle the lifetime of the
instances from creation to disposal.

In case of unmerge, the Script instance should destroy any Actor
instance, except for the Stage, and release the reference it owns. The
Stage is special because it's really owned by Clutter itself, and it
should be destroyed explicitly.

When disposing the Script itself, it should just release the reference;
any parented actor, or any InitiallyUnowned instance, will then be
managed by the parent object, as they should, while every GObject
instance will go away, as documented.

This commit is based on a patch by:

  Henrik Hedberg <hhedberg@innologies.fi>

http://bugzilla.clutter-project.org/show_bug.cgi?id=2316
This commit is contained in:
Emmanuele Bassi 2010-09-13 23:29:52 +03:00
parent 20a359cc53
commit 21eb49098a
3 changed files with 29 additions and 24 deletions

View File

@ -686,8 +686,10 @@ construct_timeline (ClutterScript *script,
_clutter_script_apply_properties (script, oinfo); _clutter_script_apply_properties (script, oinfo);
retval = CLUTTER_TIMELINE (oinfo->object); retval = CLUTTER_TIMELINE (oinfo->object);
/* we transfer ownership to the alpha function later */ /* we transfer ownership to the alpha function, so we ref before
oinfo->is_toplevel = FALSE; * destroying the ObjectInfo to avoid the timeline going away
*/
g_object_ref (retval);
object_info_free (oinfo); object_info_free (oinfo);
return retval; return retval;
@ -864,6 +866,10 @@ _clutter_script_parse_alpha (ClutterScript *script,
clutter_alpha_set_func (CLUTTER_ALPHA (retval), alpha_func, NULL, NULL); clutter_alpha_set_func (CLUTTER_ALPHA (retval), alpha_func, NULL, NULL);
clutter_alpha_set_timeline (CLUTTER_ALPHA (retval), timeline); clutter_alpha_set_timeline (CLUTTER_ALPHA (retval), timeline);
/* if we created an implicit timeline, the Alpha has full ownership
* of it now, since it won't be accessible from ClutterScript
*/
if (unref_timeline) if (unref_timeline)
g_object_unref (timeline); g_object_unref (timeline);
@ -961,9 +967,13 @@ clutter_script_parser_object_end (JsonParser *json_parser,
json_object_remove_member (object, "signals"); json_object_remove_member (object, "signals");
} }
oinfo->is_actor = FALSE;
if (strcmp (oinfo->class_name, "ClutterStage") == 0 && if (strcmp (oinfo->class_name, "ClutterStage") == 0 &&
json_object_has_member (object, "is-default")) json_object_has_member (object, "is-default"))
{ {
oinfo->is_actor = TRUE;
oinfo->is_stage = TRUE;
oinfo->is_stage_default = oinfo->is_stage_default =
json_object_get_boolean_member (object, "is-default"); json_object_get_boolean_member (object, "is-default");
@ -972,7 +982,6 @@ clutter_script_parser_object_end (JsonParser *json_parser,
else else
oinfo->is_stage_default = FALSE; oinfo->is_stage_default = FALSE;
oinfo->is_toplevel = FALSE;
oinfo->is_unmerged = FALSE; oinfo->is_unmerged = FALSE;
oinfo->has_unresolved = TRUE; oinfo->has_unresolved = TRUE;
@ -1899,13 +1908,12 @@ _clutter_script_construct_object (ClutterScript *script,
if (G_UNLIKELY (oinfo->gtype == G_TYPE_INVALID)) if (G_UNLIKELY (oinfo->gtype == G_TYPE_INVALID))
return; return;
oinfo->is_toplevel = oinfo->is_actor = g_type_is_a (oinfo->gtype, CLUTTER_TYPE_ACTOR);
g_type_is_a (oinfo->gtype, G_TYPE_INITIALLY_UNOWNED) if (oinfo->is_actor)
? FALSE oinfo->is_stage = g_type_is_a (oinfo->gtype, CLUTTER_TYPE_STAGE);
: TRUE;
} }
if (oinfo->gtype == CLUTTER_TYPE_STAGE && oinfo->is_stage_default) if (oinfo->is_stage && oinfo->is_stage_default)
{ {
GList *properties = oinfo->properties; GList *properties = oinfo->properties;
@ -1950,6 +1958,12 @@ _clutter_script_construct_object (ClutterScript *script,
params->len, params->len,
(GParameter *) params->data); (GParameter *) params->data);
/* by sinking the floating reference, we make sure that the reference
* count is correct whether the object is referenced from somewhere
* else too or only by this ClutterScript object.
*/
g_object_ref_sink (oinfo->object);
for (i = 0; i < params->len; i++) for (i = 0; i < params->len; i++)
{ {
GParameter *param = &g_array_index (params, GParameter, i); GParameter *param = &g_array_index (params, GParameter, i);

View File

@ -66,8 +66,9 @@ typedef struct {
guint merge_id; guint merge_id;
guint is_actor : 1;
guint is_stage : 1;
guint is_stage_default : 1; guint is_stage_default : 1;
guint is_toplevel : 1;
guint has_unresolved : 1; guint has_unresolved : 1;
guint is_unmerged : 1; guint is_unmerged : 1;
} ObjectInfo; } ObjectInfo;

View File

@ -314,25 +314,15 @@ object_info_free (gpointer data)
* unless we are unmerging in which case we have to destroy * unless we are unmerging in which case we have to destroy
* the actor to unparent them * the actor to unparent them
*/ */
if (oinfo->object) if (oinfo->object != NULL)
{ {
if (oinfo->is_unmerged) if (oinfo->is_unmerged)
{ {
if (oinfo->is_toplevel) if (oinfo->is_actor && !oinfo->is_stage)
g_object_unref (oinfo->object);
else
{
/* destroy every actor, unless it's the default stage */
if (oinfo->is_stage_default == FALSE &&
CLUTTER_IS_ACTOR (oinfo->object))
clutter_actor_destroy (CLUTTER_ACTOR (oinfo->object)); clutter_actor_destroy (CLUTTER_ACTOR (oinfo->object));
} }
}
else
{
if (oinfo->is_toplevel)
g_object_unref (oinfo->object); g_object_unref (oinfo->object);
}
oinfo->object = NULL; oinfo->object = NULL;
} }