Add an xpidl attribute to mark an interface as 'Sync' in rust
Categories
(Core :: XPCOM, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox117 | --- | fixed |
People
(Reporter: nika, Assigned: nika)
Details
Attachments
(5 files, 1 obsolete file)
Currently rust-xpcom marks all generated interface types as non-send and non-sync to reflect the fact that these interfaces are likely not safe to send across threads. However, there are some interfaces which are generally safe to send across threads, and should be Send and Sync because all implementations use a threadsafe implementation.
We should have an attribute we can put on the XPCOM interface to indicate that it satisfies Sync requirements, and relaxes the codegen on the Rust side to match. Ideally this would also come with assertions on the C++ side to ensure that every class implementing the interface is actually at least using threadsafe reference counting.
|
Assignee | |
Comment 1•2 years ago
|
||
This attribute indicates that all implementations of the interface must
implement the Sync trait in Rust, meaning that they're safe to share between
threads. This will make storing these values in threadsafe types in Rust more
ergonomic.
To implement this, the vtable types were changed to be &'static references
rather than raw pointers. This should be OK as they are always valid non-null
pointers to the VTable, and avoids the need for manual unsafe impls for the
relevant interfaces.
Other interfaces will remain non-send and non-sync due to the marker type.
|
|
Assignee | |
Comment 2•2 years ago
|
||
This adds c++-side assertions to annotated interfaces, requiring them to use a
threadsafe reference count. This ended up being quite gross due to interface
table entries making it difficult to add a static_assert into the code, so it
ended up being done through a constexpr constant which validates before
returning the IID.
Depends on D183590
|
|
Assignee | |
Comment 3•2 years ago
|
||
All event targets should be threadsafe and implemented in C++, and so should be
able to be used in Sync types in Rust code.
This also required annotating all interfaces deriving from nsIEventTarget, as
well as adding some associated constants to specific types to indicate to the
static assertion that they have threadsafe reference counts.
Depends on D183591
|
|
Assignee | |
Comment 4•2 years ago
|
||
|
|
Assignee | |
Comment 5•2 years ago
|
||
This attribute promises that the interface is threadsafe, and validates it with
some assertions in generated code. As these interfaces were already only
implemented in Rust code, and always threadsafe, the assertions were easy to
add as a example use-case for the new attributes.
|
|
Assignee | |
Comment 6•2 years ago
|
||
I'm not sure if this is a thing we'd want to start using outside of
clang-plugins, as it's very internal-clang-specific. This does appear to
work however.
The logic is derived from the testing framework we use for clang-plugin
testing.
|
||
Updated•2 years ago
|
|
||
Comment 8•2 years ago
•
|
||
Backed out 5 changesets (Bug 1843568) for causing xpc failures in test_extension_permissions_migration.js CLOSED TREE
Log: https://treeherder.mozilla.org/logviewer?job_id=423197203&repo=autoland&lineNumber=3212
https://treeherder.mozilla.org/logviewer?job_id=423200380&repo=autoland&lineNumber=5272
https://treeherder.mozilla.org/logviewer?job_id=423200896&repo=autoland&lineNumber=2269
Backout: https://hg.mozilla.org/integration/autoland/rev/a7d728e9e7bc6c8f8f6c4aba1d2245c8f980d4fe
|
||
Comment 10•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/0fb686987260
https://hg.mozilla.org/mozilla-central/rev/322cee5ec13d
https://hg.mozilla.org/mozilla-central/rev/4e50ed5a7a1f
https://hg.mozilla.org/mozilla-central/rev/a6691f035a70
https://hg.mozilla.org/mozilla-central/rev/2aad3896d42a
|
|
Assignee | |
Updated•2 years ago
|
Description
•