On 05/02/16 08:50 AM, Derek Foreman wrote: > On 05/02/16 07:25 AM, Pekka Paalanen wrote: >> From: Pekka Paalanen <[email protected]> >> >> Since shm_pool_resize() uses mremap(MREMAP_MAYMOVE), the pool's base >> address may change at that point. >> >> If a compositor stores the pointer and a client enlarges the pool, the >> compositor will have a stale pointer. >> >> Signed-off-by: Pekka Paalanen <[email protected]> > > Looks good to me, > Reviewed-by: Derek Foreman <[email protected]>
Been thinking about this lately, and this *really* screws up Enlightenment's asynchronous renderer - and I guess anyone else that tries to render from a thread while dispatching in another. If we don't have any way to prevent this from happening out from under us, our renderer can be killed intentionally by any client - this is not good. Even the shm pool reference counting functions I added don't give us a way out of this right now, as a resize will still change all the pointers we've gathered before firing off the async rendering thread. Any suggestions on resolving this? I'd be happy changing the pool resize semantics to only allow it when there are no unreleased buffers in the pool. (or only allow it when the compositor doesn't have a reference - but unreleased buffers would be the only way a client could infer that...) Actually, I wouldn't be surprised if the only user of pool resize right now is libwayland-cursor, and it does all its resizing before it ever submits a buffer - I'd be quite happy to make that the required way to use resize. ;) We really *really* need to be able to store pointers over dispatch to do async rendering, so while this has never actually been possible, it has at least not been documented as impossible yet. I think I need to retract this too-speedy Reviewed-by... :/ >> --- >> src/wayland-shm.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/src/wayland-shm.c b/src/wayland-shm.c >> index a4343a4..7e42dcb 100644 >> --- a/src/wayland-shm.c >> +++ b/src/wayland-shm.c >> @@ -348,6 +348,10 @@ wl_shm_buffer_get_stride(struct wl_shm_buffer *buffer) >> * to crash you should call wl_shm_buffer_begin_access and >> * wl_shm_buffer_end_access around code that reads from the memory. >> * >> + * @note The return value of this function must not be stored across >> + * dispatching client requests. If a client resizes the underlying shm pool, >> + * the resize request handler will remap, and the pool base address may >> change. >> + * >> * \memberof wl_shm_buffer >> */ >> WL_EXPORT void * >> > > _______________________________________________ > wayland-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
