Merge lp://staging/~vanvugt/compiz-core/fix-943116 into lp://staging/compiz-core

Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: 3030
Merged at revision: 3030
Proposed branch: lp://staging/~vanvugt/compiz-core/fix-943116
Merge into: lp://staging/compiz-core
Diff against target: 127 lines (+15/-53)
2 files modified
plugins/decor/src/decor.cpp (+13/-52)
plugins/decor/src/decor.h (+2/-1)
To merge this branch: bzr merge lp://staging/~vanvugt/compiz-core/fix-943116
Reviewer Review Type Date Requested Status
Sam Spilsbury Approve
Review via email: mp+95128@code.staging.launchpad.net

Description of the change

Revert enhancement LP: #936784 (revisions 3027,3028) because it causes
invalid memory accesses (use after free) (LP: #943116)

To post a comment you must log in.
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

See my comment on https://bugs.launchpad.net/compiz-core/+bug/943116

We just need to not use CompRegionRef non-temporary in this case.

I also think its a good idea to add constructors which for CompRect & and CompRegion & to make sure that this kind of stuff can't happen.

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

err, use a CompRegion, not a ref, rather.

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Incidentally, the optimizations I did for borderRect* etc will also fix this.

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

On Wed, 29 Feb 2012, Daniel van Vugt wrote:

Could you have a look at

https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.optimization-inlining/+merge/95094]

It does break the ABI however it will boost performance and also fix this
issue.

> The proposal to merge lp:~vanvugt/compiz-core/fix-943116 into lp:compiz-core has been updated.
>
> Status: Needs review => Work in progress
>
> For more details, see:
> https://code.launchpad.net/~vanvugt/compiz-core/fix-943116/+merge/95128
> --
> https://code.launchpad.net/~vanvugt/compiz-core/fix-943116/+merge/95128
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~vanvugt/compiz-core/fix-943116 into lp:compiz-core.
>

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The bug has nothing to do with CompRegionRef. It keeps happening even after I removed all CompRegionRefs and made them CompRegions. The actual problem appears to be invalid data in screen->clientList ().

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I recommend we do this simple revert, and look at a more stable fix for bug 936784 later (0.9.7.2, after 0.9.7.0).

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Fine, revert it then.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I may have spoken too soon. There might be a simple fix here.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches