LLM summary:
func only ever needs to be moved to a thread and called there sequentially, never shared by & across threads:
- In the erasure, func is captured by-move into the Box<dyn Fn ... + Send> callback (only Send is required to box it that way).
- The Python pool moves that callback into the runInteraction DB-thread closure and calls it there; the Rust pool calls it within a single async task. Neither sends &func to multiple threads concurrently.
Prompt:
```
Compare this with a previous approach that was swapped out in `d9a111bdfaf51cd4cadfdc071f9956eec47aea43` (look at the removals). Anything we should change/simplify or something you like better/worse about that one? Give the new approach a review and simplify and make things more clear as necessary
```
Changes made (LLM summary):
1. #[async_trait] on the object-safe method. The core trait method is now async fn run_interaction_erased(...) -> ErasedResult instead of a hand-rolled fn ... -> BoxFuture<'a, _>. This is the convention already used by Transaction in the same file, and it removed the async move { ... }.boxed() wrapper (plus the BoxFuture/FutureExt imports) from both pool impls — their bodies are now plain async fns again.
2. Dropped the constrain / erase_interaction helper. I'd over-engineered the closure erasure out of a worry that higher-ranked-lifetime inference would fail. The previous code proved a direct let erased: ErasedInteraction = Box::new(move |txn| {...}) compiles — the type annotation on the binding is enough. ~30 lines gone.
3. Dropped the unneeded + Sync on the erased callback type. ErasedInteraction is + Send only now; the boxed callback is moved to a single DB thread and called there, never shared by reference.
```
[ERROR]
Traceback (most recent call last):
File "/home/eric/.cache/pypoetry/virtualenvs/matrix-synapse-xCtC9ulO-py3.14/lib/python3.14/site-packages/twisted/trial/runner.py", line 711, in loadByName
return self.suiteFactory([self.findByName(name, recurse=recurse)])
File "/home/eric/.cache/pypoetry/virtualenvs/matrix-synapse-xCtC9ulO-py3.14/lib/python3.14/site-packages/twisted/trial/runner.py", line 474, in findByName
obj = reflect.namedModule(searchName)
File "/home/eric/.cache/pypoetry/virtualenvs/matrix-synapse-xCtC9ulO-py3.14/lib/python3.14/site-packages/twisted/python/reflect.py", line 156, in namedModule
topLevel = __import__(name)
File "/home/eric/Documents/github/element/synapse/tests/__init__.py", line 24, in <module>
from synapse.util.patch_inline_callbacks import do_patch
File "/home/eric/Documents/github/element/synapse/synapse/__init__.py", line 31, in <module>
from synapse.util.rust import check_rust_lib_up_to_date
File "/home/eric/Documents/github/element/synapse/synapse/util/__init__.py", line 41, in <module>
from synapse.types import JsonDict
File "/home/eric/Documents/github/element/synapse/synapse/types/__init__.py", line 63, in <module>
from synapse.api.errors import Codes, SynapseError
File "/home/eric/Documents/github/element/synapse/synapse/api/errors.py", line 33, in <module>
from synapse.util.json import json_decoder
File "/home/eric/Documents/github/element/synapse/synapse/util/json.py", line 23, in <module>
from synapse.synapse_rust.events import JsonObject
File "/home/eric/Documents/github/element/synapse/synapse/logging/context.py", line 55, in <module>
from synapse.util.stringutils import random_string_insecure_fast
File "/home/eric/Documents/github/element/synapse/synapse/util/stringutils.py", line 31, in <module>
from synapse.api.errors import Codes, SynapseError
builtins.ImportError: cannot import name 'Codes' from partially initialized module 'synapse.api.errors' (most likely due to a circular import) (/home/eric/Documents/github/element/synapse/synapse/api/errors.py)
```
This is in prep for converting the event serialization to Rust.
This is a fairly mechanical port, except that we store the appservice ID
rather than the appservice object. This avoids us having to store a
`Py<..>` (or port the appservice object over).
Follow on from #19701
This (more or less) matches what we had before. Otherwise we just get a
default `<builtins.Event at 0x...>`
---------
Co-authored-by: Eric Eastwood <erice@element.io>
Follow on from https://github.com/element-hq/synapse/pull/19701.
Unfortunately serde has a bug when using `#[serde(flatten)]` with
`arbitrary-precision` feature when handling integers that fit in a i128
when doing `serde_json::from_value`. See
https://github.com/serde-rs/serde/issues/2230.
The `depythonize` hits the same issue. To fix this we make it so we only
parse events from strings and not values.
Follow on from #19701.
Some Synapse servers may have events in their database that don't pass
the canonical JSON checks. This is bad, but we still want to be able to
load them nonetheless.
Ports the event class to Rust.
The main difference here are:
1. There is now a single event class
2. We now validate a lot more at event construction time than we
previously did (we basically checked nothing before). This required some
changes to the tests, including
https://github.com/matrix-org/sytest/pull/1423
Reviewable commit-by-commit.
### Overview of Event Rust structure
The format of the event struct in Rust is quite different than that in
Python.
The top-level looks like:
```rust
pub struct Event {
/// The parsed event JSON.
fields: FormattedEvent,
/// The event ID. For format v1 this is read directly from the JSON;
/// for v2+ it is computed from the canonical-JSON hash at
/// construction time and cached here.
event_id: Arc<str>,
/// Synapse-internal per-event state that lives outside the federated
/// JSON (e.g. outlier flag, soft-failure, stream positions).
#[pyo3(get)]
internal_metadata: EventInternalMetadata,
/// The room version this event was parsed for.
#[pyo3(get)]
room_version: &'static RoomVersion,
/// `None` for accepted events; otherwise a short reason set by auth
/// when the event was rejected.
rejected_reason: Option<Box<str>>,
}
```
which includes the actual parsed event in `FormattedEvent`, plus the
rest of the event metadata.
```rust
pub struct FormattedEvent<E = Arc<EventFormatEnum>> {
#[serde(default)]
pub signatures: Signatures,
#[serde(default)]
pub unsigned: Unsigned,
#[serde(flatten)]
pub specific_fields: E,
#[serde(flatten)]
pub common_fields: Arc<EventCommonFields>,
}
```
The struct is further split into the common fields, format specific
fields, plus the signatures and unsigned. We split out the signature and
unsigned fields as they are mutable, so when we clone the event we can
still share the common and specific fields and only copy signature and
unsigned.
The `specific_fields` are the fields that depend on the format version.
They can either be a specific format (e.g. `E = EventFormatV1`) or a
type-erased enum `EventFormatEnum` that is across all room versions:
```rust
pub enum EventFormatEnum {
V1(EventFormatV1),
V2V3(EventFormatV2V3),
V4(EventFormatV4),
VMSC4242(EventFormatVMSC4242),
}
```
For example:
```rust
/// Shared flat-list encoding of `auth_events` and `prev_events`, reused
/// by every format from v2/v3 onwards.
#[derive(Serialize, Deserialize)]
pub struct SimpleAuthPrevEvents {
pub auth_events: Vec<String>,
pub prev_events: Vec<String>,
}
/// Version-specific fields for room versions 3-10.
#[derive(Serialize, Deserialize)]
pub struct EventFormatV2V3 {
pub room_id: Box<str>,
#[serde(flatten)]
pub auth_prev_events: SimpleAuthPrevEvents,
}
```
### Dev notes
As discussed in
[`#element-backend-internal:matrix.org`](https://matrix.to/#/!SGNQGPGUwtcPBUotTL:matrix.org/$3gTjDO440GbAz57cXcCawwiyFLiD0crrarvS1uhzKOY?via=jki.re&via=element.io&via=matrix.org)
---------
Co-authored-by: Eric Eastwood <erice@element.io>
This is in prep for using the room versions more from Rust.
Main changes:
- Change it so each room version is defined as a delta to the last one.
This is a cosmetic change that makes it easier to ensure the room
version definitions are correct (as they're defined as deltas from
previous versions).
- Move constants to `RoomVersion` constants, like `RoomVersion::V1`, for
convenience.
- Change visibility of various attributes.
Based on #19708.
This is on the path to porting the entire event class to Rust, as
`event.content` will then return the new Rust class `JsonObject`.
This PR adds a pure Rust `JsonObject` class that is a `Mapping`
representing a json-style object. It uses `serde_json::Value` as its
in-memory representation and `pythonize` for conversion when a field is
looked up on the object.
I'm not thrilled with the name, but couldn't think of a better one.
This also adds `JsonObject` handling to the JSON serialisation functions
we use, as well as to the `freeze(..)` function.
Reviewable commit-by-commit.
Similar to #19706, let's port the `unsigned` field into a Rust class.
This does change things a bit in that we now define exactly what
unsigned fields that are allowed to be added to an event, and what
actually gets persisted. This should be a noop though, as we carefully
filter out what unsigned fields we allow in from federation, for example
As a side effect of this cleanup, I think this fixes handling
`unsigned.age` on events received over federation.
This is another stepping stone in porting the event class fully to Rust.
The new `Signatures` class is relatively simple, as we actually don't
interact with it that much in the code. It does *not* implement
`Mapping` or `MutableMapping` as that takes quite a lot of effort that
we don't need, even though it would be more ergonomic.
This comes from
https://github.com/erikjohnston/rust-signed-json/blob/main/src/json.rs.
We need to be able to serialise canonical JSON in Rust to be able to
calculate event IDs once we port the event class to Rust.
We could instead make the above a properly published crate, but feels
easier to pull it into Synapse utils.
Eric Eastwood