Hi,

Thanks for getting to this. I was waiting for the release, but I'm currently 
not at full capacity, so you got it before me.

The commit message of patch 1 contains a lie. The second paragraph should 
contain "IF there was only one listener object", which the testcase in Patch 3 
shows. But I don't think that's a deal breaker.

For Patch 1/2:
Reviewed-by: Markus Ongyerth <w...@ongy.net>

I'm fine with moving/resubmit of my code and am happy I could provide the 
testcase that shows an issue.
Since it's originally authored by me, I guess my R-B would be weird there :)

Cheers,
ongy

On 2018/April/16 03:00, Derek Foreman wrote:
> Now that the release is out, I'd like to dig back into this mess.
> This is a round up of some patches that were on list shortly before
> the release to deal with a problem where many existing libwayland
> users don't delete their destroy signal listeners before freeing
> them.
> 
> These leads to a bit of a mess (as Markus' test illustrates) if there
> are multiple destroy listeners.
> 
> I've included:
> My test patch to ensure the existing behaviour continues to work
> (users like weston and enlightenment can free during destroy
> listener)
> 
> The special case destroy emit path for wl_priv_signal - this is
> an attempt to "fix" the problem, by making the destroy signal
> emit operate without ever touching potentially free()d elements
> again.
> 
> Markus' test that would fail without patch 2/3, as it catches the
> free() without removal case we've all come to know any love.
> 
> Derek Foreman (2):
>   tests: Test for use after free in resource destruction signals
>    changes since first appearance: none
> 
>   server: Add special case destroy signal emitter
>    changes since first appearance:  stop trying to maintain a list head
> 
> Markus Ongyerth (1):
>   tests: Add free-without-remove test
>    changes since first appearance: I moved it into an existing file
> 
>  src/wayland-private.h  |  3 +++
>  src/wayland-server.c   | 46 +++++++++++++++++++++++++++++++++++++++++++---
>  tests/resources-test.c | 39 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 85 insertions(+), 3 deletions(-)
> 
> -- 
> 2.14.3
> 
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to