Open Bug 1651053 Opened 4 years ago Updated 3 years ago

WebRenderBridgeParent can be destroyed on scene builder thread, violating WeakPtr threading assertions

Categories

(Core :: Graphics: WebRender, defect, P3)

defect

Tracking

()

People

(Reporter: kats, Assigned: nical)

References

Details

Attachments

(2 obsolete files)

Seen on a try push:

[task 2020-07-07T11:46:51.251Z] 11:46:51     INFO - REFTEST TEST-LOAD | http://localhost:50186/1594122129537/4/579808-1.html | 1499 / 2052 (73%)
[task 2020-07-07T11:46:51.252Z] 11:46:51     INFO - [Child 1543, Main Thread] WARNING: NS_ENSURE_TRUE(browserChrome) failed: file /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp, line 11384
[task 2020-07-07T11:46:51.283Z] 11:46:51     INFO - Assertion failure: false (WeakPtr accessed from multiple threads), at /builds/worker/workspace/obj-build/dist/include/mozilla/WeakPtr.h:167
[task 2020-07-07T11:46:51.283Z] 11:46:51     INFO - #01: mozilla::layers::WebRenderBridgeParent::~WebRenderBridgeParent() [gfx/layers/wr/WebRenderBridgeParent.cpp:382]
[task 2020-07-07T11:46:51.284Z] 11:46:51     INFO - #02: non-virtual thunk to mozilla::layers::WebRenderBridgeParent::~WebRenderBridgeParent() [gfx/layers/wr/WebRenderBridgeParent.cpp:0]
[task 2020-07-07T11:46:51.285Z] 11:46:51     INFO - #03: mozilla::layers::SceneBuiltNotification::~SceneBuiltNotification() [gfx/layers/wr/WebRenderBridgeParent.cpp:212]
[task 2020-07-07T11:46:51.285Z] 11:46:51     INFO - #04: webrender_api::api::NotificationRequest::notify [gfx/wr/webrender_api/src/api.rs:2063]
[task 2020-07-07T11:46:51.285Z] 11:46:51     INFO - #05: webrender::scene_builder_thread::SceneBuilderThread::process_transaction [gfx/wr/webrender/src/scene_builder_thread.rs:744]
[task 2020-07-07T11:46:51.285Z] 11:46:51     INFO - #06: webrender::scene_builder_thread::SceneBuilderThread::run [gfx/wr/webrender/src/scene_builder_thread.rs:342]
[task 2020-07-07T11:46:51.293Z] 11:46:51     INFO - #07: std::sys_common::backtrace::__rust_begin_short_backtrace [git:github.com/rust-lang/rust:src/libstd/sys_common/backtrace.rs:4fb7144ed159f94491249e86d5bbd033b5d60550:130]
[task 2020-07-07T11:46:51.294Z] 11:46:51     INFO - #08: std::panicking::try::do_call [git:github.com/rust-lang/rust:src/libstd/panicking.rs:4fb7144ed159f94491249e86d5bbd033b5d60550:305]
[task 2020-07-07T11:46:51.588Z] 11:46:51     INFO - [GFX1-]: Receive IPC close with reason=AbnormalShutdown

Looks like the SceneBuiltNotification holds a refptr to the WRBP, and that ends up being the last strong pointer to it, so when the scene builder thread unwinds it deletes the WRBP. Which doesn't play well with WeakPtrs, because apparently that is not threadsafe.

As far as I can tell WRBP itself doesn't hold WeakPtrs to anything, but there is a WeakPtr to the WRBP, here.

Blocks: gfx-triage
Severity: -- → S3
Priority: -- → P3

Nical - can you take a peek at this?

Flags: needinfo?(nical.bugzilla)
Blocks: wr-80
No longer blocks: gfx-triage
Assignee: nobody → nical.bugzilla
Flags: needinfo?(nical.bugzilla)

This tastes more like a workaround than a satisfying fix for having strong references to webrender WRBP moving around threads but a better fix would probably need to be quite a bit more involved.

Blocks: wr-81
No longer blocks: wr-80
No longer blocks: wr-81
No longer blocks: gfx-82

I think we should make the IPC actor inherit from the now available ThreadSafeWeakPtr. This artificial limitation on where an actor must be destroyed or referenced from is not particularly useful.

Nika, do you have an opinion on the matter?

Flags: needinfo?(nika)

That seems fine, especially if the type is already generally threadsafe.

Give PWebRenderBridge is already reference counted, it would be nice to mark it as a refcounted actor in IPDL as well.

Flags: needinfo?(nika)
No longer blocks: gfx-83
See Also: → 1676505
Depends on: 1570771
Depends on: 1676835
No longer depends on: 1570771
No longer depends on: 1676835
Attachment #9187355 - Attachment is obsolete: true
Attachment #9164028 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: