Bug 223211 - Protect this in all PlatformOpenXR queue->dispatch() calls
Summary: Protect this in all PlatformOpenXR queue->dispatch() calls
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebXR (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Imanol Fernandez
Keywords: InRadar
Depends on:
Blocks: 208988
  Show dependency treegraph
Reported: 2021-03-15 13:25 PDT by Imanol Fernandez
Modified: 2021-03-16 01:54 PDT (History)
3 users (show)

See Also:

Patch (9.11 KB, patch)
2021-03-15 13:37 PDT, Imanol Fernandez
no flags Details | Formatted Diff | Diff
Patch (9.04 KB, patch)
2021-03-15 13:55 PDT, Imanol Fernandez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Imanol Fernandez 2021-03-15 13:25:36 PDT
As discussed in https://bugs.webkit.org/show_bug.cgi?id=222607, the WorkQueue may outlive OpenXRDevice so m_queue->dispatch([this] { /*do something with this*/}); is unsafe.
Comment 1 Imanol Fernandez 2021-03-15 13:37:49 PDT
Created attachment 423231 [details]
Comment 2 Chris Dumez 2021-03-15 13:42:23 PDT
Comment on attachment 423231 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=423231&action=review

> Source/WebCore/platform/xr/PlatformXR.h:65
> +class Device : public RefCounted<Device>, public CanMakeWeakPtr<Device> {

You want ThreadSafeRefCounted since you are passing this to another thread.

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h:50
> +    virtual ~OpenXRDevice();

I don't think we need the `virtual`.
Comment 3 Imanol Fernandez 2021-03-15 13:55:00 PDT
Created attachment 423233 [details]

Thanks for the quick review! I added ThreadSafeRefCounted and removed the unneeded virtual destructor in the inherited class
Comment 4 Chris Dumez 2021-03-15 14:00:07 PDT
Comment on attachment 423233 [details]

r=me if the bots are happy.
Comment 5 EWS 2021-03-16 01:45:56 PDT
Committed r274470: <https://commits.webkit.org/r274470>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423233 [details].
Comment 6 Radar WebKit Bug Importer 2021-03-16 01:46:15 PDT