[PATCH wayland] shm: add a note about shm pool base address changing

[Derek Foreman]

On 05/02/16 08:50 AM, Derek Foreman wrote:
> On 05/02/16 07:25 AM, Pekka Paalanen wrote:
>> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>>
>> 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 <pekka.paalanen at collabora.co.uk>
> 
> Looks good to me,
> Reviewed-by: Derek Foreman <derekf at osg.samsung.com>

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
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>