[Webkit-unassigned] [Bug 222607] Implement WebXR Opaque Framebuffer

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=222607

--- Comment #13 from Imanol Fernandez <ifernandez at igalia.com> ---
(In reply to youenn fablet from comment #11)
> > > What guarantees |this| is still alive when the lambda runs on the work queue. This looks unsafe.
> > 
> > Good point. I used the same pattern in this PR as existing code in other
> > calls. Checking this more, PlatformXR::Instance is LazyNeverDestroyed and
> > it's holding both the queue and the device instance. In the case the device
> > is destroyed the queue would be destroyed at the same time, with supposedly
> > cancelled queue callbacks. But I see some other risks too if we implement
> > device enumeration changes in the future, reusing the queue but with a
> > different device.
> > 
> > To make the code safer I added a weakThis check in the queue lambda. If you
> > like this approach I'll create a follow-up issue to apply the same pattern
> > in all the other m_queue.dispatch methods and to revisit the destruction of
> > the instance.
> 
> The usual pattern is to have the object owning the WorkQueue. Can we do that
> here or is there a need for a single queue encompassing all objects?
> 
> Weak pointers and work queues do not tend to work well together, checking a
> weak pointer is usually only safe in one specific thread.

PlatformOpenXR has a single queue (In reply to youenn fablet from comment #11)
> > > What guarantees |this| is still alive when the lambda runs on the work queue. This looks unsafe.
> > 
> > Good point. I used the same pattern in this PR as existing code in other
> > calls. Checking this more, PlatformXR::Instance is LazyNeverDestroyed and
> > it's holding both the queue and the device instance. In the case the device
> > is destroyed the queue would be destroyed at the same time, with supposedly
> > cancelled queue callbacks. But I see some other risks too if we implement
> > device enumeration changes in the future, reusing the queue but with a
> > different device.
> > 
> > To make the code safer I added a weakThis check in the queue lambda. If you
> > like this approach I'll create a follow-up issue to apply the same pattern
> > in all the other m_queue.dispatch methods and to revisit the destruction of
> > the instance.
> 
> The usual pattern is to have the object owning the WorkQueue. Can we do that
> here or is there a need for a single queue encompassing all objects?
> 
> Weak pointers and work queues do not tend to work well together, checking a
> weak pointer is usually only safe in one specific thread.

We need to initalize the OpenXR XrInstance in the queue first, before device enumeration and device instance creation. We can take a Ref<WorkQueue> in the device to protect it from destruction while the device is alive

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210308/e8f63c98/attachment.htm>