GNOME Bugzilla – Bug 683414
memory corruption when freeing layer cache
Last modified: 2012-09-07 10:49:09 UTC
I am attaching a patch to fix a memory corruption due to attempting to free a zero-size layer cache; run into this with clutter on Beagleboard.
Created attachment 223525 [details] [review] Patch to fix memory corruption
I'm not really sure how it can get into this situation because recursively_free_layer_caches immediately bails out if pipeline->layers_cache_dirty==TRUE. As far as I can see the only way that can become FALSE is in _cogl_pipeline_update_layers_cache. That function doesn't do anything if pipeline->n_layers==0 so presumably in that case layers_cache_dirty will always be TRUE? Maybe there is something more to this bug that is causing it to get into an inconsistent state?
I am hitting the code paths with no HW repeat, i.e., seeing lot of (media-explorer:703): Cogl-WARNING **: Disabling layer 0 of the current source material, because texturing with the vertex buffer API is not currently supported using sliced textures, or textures with waste (media-explorer:703): Cogl-WARNING **: Skipping layers 1..n of your material since the first layer doesn' t support hardware repeat (e.g. because of waste or use of GL_TEXTURE_RECTANGL E_ARB) and you supplied texture coordinates outside the range [0,1].Falling ba ck to software repeat assuming layer 0 is the most important one keep I imagine the 0-size cache is related to this.
Disabling a layer will end up calling through to _cogl_pipeline_prune_to_n_layers. It looks like there are a couple of issues there which might cause it to get into this state. I'm attaching two patches which might help. I haven't tested them in Tomas' use case though.
Created attachment 223535 [details] [review] pipeline: Ensure the pipeline layer cache is freed when pruning layers When pruning layers from a pipeline the pipeline cache would once be freed due to the call to pre_change_notify but it would immediately be recreated again when foreach_layer_internal is called. When n_layers is later set to 0 it would end up with an invalid cache lying around. This patch changes the order so that it will iterate the layers first before triggering the pre-change notify so that the cache will be cleared correctly.
Created attachment 223536 [details] [review] pipeline: Fix the layer index used when pruning layers When pruning a pipeline to a set number of layers it records the index of the first layer after the given number of layers have been found. This is stored in a variable called 'first_index_to_prune' implying that this layer should be included in the layers to be pruned. However the subsequent if-statement was only pruning layers with an index greater than the recorded index so it would presumably only prune the following layers. This patch fixes it to use '>=' instead.
I can confirm Neil's two patches fix the memory corruption issue.
Review of attachment 223535 [details] [review]: This patch looks good to land to me, though I think perhaps it would be good to put a comment above the _pre_change_notify to explain briefly which it isn't done first. thanks
Review of attachment 223536 [details] [review]: this patch looks good to land to me
Ok, I've added a comment as suggested and pushed both patches to all three branches. http://git.gnome.org/browse/cogl/commit/?id=1c8efdc838cc5ace380365c http://git.gnome.org/browse/cogl/commit/?id=d3063e8dea92a8f668acef6