Closes#20116
> [!NOTE]
> This PR Follows #20115 which already implemented this for
`m.device_list_update`
> User IDs with localparts containing characters outside the range
U+0021 to U+007E, or with
> an empty localpart, are considered non-compliant. For current room
versions, servers must
> still accept events using such user IDs over federation; however they
SHOULD NOT forward
> such user IDs to clients when referenced outside the context of an
event. For example,
> device list updates from non-compliant user IDs would be dropped by
the receiving server.
>
> — [Spec
1.19](https://spec.matrix.org/v1.19/appendices/#historical-user-ids)
This PR prevents from forwarding non compliant user ids for the
following events:
- `m.signing_key_update` 8c3c35e95d
- `m.presence` 26a8dfe862
- `m.typing` 1110cf2d73
- `m.receipt` 9e2da8b0a6
- `m.direct_to_device` 9541232a30
These things are not needed now that MSC2965 has stabilised. In any
case, they
should never have been exposed without a config flag in the first place.
This speeds up the /createRoom calls a tiny bit by not checking the
senders push rules.
This helps speed up a lot of test runs in Element Web which
runs a lot of tests in one-person rooms (and creates a heck of a lot of rooms)
## Summary
- default `request_token_inhibit_3pid_errors` to `true` to prevent
account enumeration
- document the new default and the explicit opt-out
- add an upgrade note and regression coverage for the default
Fixes#18415.
Part of: MSC4354
Part of: #19641
Follows: #20232 (which applies already-persisted redactions to Sticky
Events being persisted)
In some cases we batch events together during persistence.
If one of the events in the batch is a redaction of another event, we
should apply it, which provides the same benefit as #20232 (namely: we
avoid potentially dodgy event content being persisted & the event
naturally loses its stickiness
so is not written to `sticky_events`)
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Fixes#20309.
For endpoints that validate the request body with Pydantic, a validation
failure put Pydantic's raw verbose error text into the `error` field of
the standard error response, e.g.:
```json
{
"errcode": "M_MISSING_PARAM",
"error": "1 validation error for PutBody\\nroom_id\\n Field required [type=missing, input_value={}, input_type=dict]\\n For further information visit https://errors.pydantic.dev/2.12/v/missing"
}
```
This leaks internal details clients cannot act upon (model class names,
raw input values, pydantic.dev URLs). This PR adds a
`_format_pydantic_error` helper in `synapse/http/servlet.py` that builds
a short client-friendly message from the structured
`ValidationError.errors()` data instead of `str(e)`:
- `{}` against a model requiring `room_id` -> `Missing required field
'room_id'.`
- `{"room_id": 123}` -> `'room_id': Input should be a valid string.`
- nested models use dotted paths, e.g. `Missing required field
'put.room_id'.`
- multiple errors are joined: `Missing required field 'name'; Missing
required field 'put'.`
The `errcode` selection logic (`M_MISSING_PARAM` / `M_INVALID_PARAM` /
`M_BAD_JSON`) is unchanged.
Part of #19414 (Support Matrix 1.16).
⚠️ Before merging, wait for the following PRs to be merged first:
- https://github.com/element-hq/synapse/pull/20081
- https://github.com/element-hq/synapse/pull/20169
- https://github.com/element-hq/synapse/pull/20171
---
Since Matrix 1.16, the
[spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv3sync)
defines the stable identifiers for
[MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222):
the `use_state_after` query parameter on `GET /_matrix/client/v3/sync`,
and the `state_after` room response field:
> **`use_state_after`** — `boolean` — Controls whether to receive state
changes between the previous sync and the start of the timeline, or
between the previous sync and the end of the timeline. If this is set to
`true`, servers MUST respond with the state between the previous sync
and the end of the timeline in `state_after` and MUST omit `state`. […]
>
> **`state_after`** — […] Required if `use_state_after` was set to true
in the request, even if it is empty. Added in `v1.16`
>
> — [Matrix
Spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv3sync)
Synapse so far only implemented the unstable identifiers
(`org.matrix.msc4222.use_state_after` /
`org.matrix.msc4222.state_after`), gated behind the `msc4222_enabled`
experimental flag and the per-user experimental features admin API.
This PR:
- accepts the stable `use_state_after` query parameter, unconditionally,
and responds with the stable `state_after` field. The unstable query
parameter is kept for a transition period; clients opting in with it get
the unstable response field name back, so unstable-era clients see an
unchanged wire format. If both parameters are set, the stable one wins.
- removes the `msc4222_enabled` experimental flag and the `msc4222`
per-user feature from the experimental features admin API, since the
behaviour is no longer gated.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Fixes https://github.com/element-hq/synapse/issues/18793
When the last local user leaves a room, Synapse clears
`current_state_events` and writes `null` `event_id` entries into the
`current_state_delta_stream`. The incremental sync code for MSC4222's
state_after skipped those `null` deltas entirely, and
`get_current_state_ids` also returned nothing (table already cleared).
Result: clients using `state_after` never learned about the leave.
Fix by falling back to state groups (which are preserved) to look up the
actual state at `end_token` for any cleared state keys.
### Pull Request Checklist
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Paul Chobert <paul@chobert.fr>
Co-authored-by: Quentin Gliech <quenting@element.io>
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Several clients, including Element Web/Desktop and Element X, have
already implemented
[MSC4426](https://github.com/matrix-org/matrix-spec-proposals/pull/4426)
User Status profile fields. However, we have only implemented sending
profile fields down sync for local users
([MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429)/[MSC4262](https://github.com/matrix-org/matrix-spec-proposals/pull/4262)),
since federation support for pushing profile field updates from server
to server has not yet been implemented
([MSC4259](https://github.com/matrix-org/matrix-spec-proposals/pull/4259)).
This has created user confusion. The before mentioned clients use the
profile and/or profile field fetch endpoints to fetch missing profile
information, regardless of the user being local or remote. Since the
sync response isn't delivering these profiles, the client does a
fallback fetch, causing a one time update of the profile information
over federation.
This causes stale data to be cached in the client for the remote user,
which in the case of EW for example is never cleared.
This filtering should be temporary until MSC4259 is implemented and the
remote profile data also syncs via the profile update stream to clients.
I suggest making this not configurable so we don't have to maintain a
configuration property, as hopefully this would only be a case for a few
months at most.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
TLDR: `PUT` on a server-default push rule now applies the same existence
check that `GET` already does. A rule that is not served cannot be
modified.
Synapse ships some server-default push rules behind experimental feature
flags, so the set of server-default rules a user has depends on the
server's configuration. Most flags add rules that only exist while the
flag is on.
[MSC4210](https://github.com/matrix-org/matrix-spec-proposals/pull/4210)
works the other way: it removes the three legacy mention rules while its
flag is on. The flags that change the set are:
- `msc1767_enabled`:
[MSC1767](https://github.com/matrix-org/matrix-spec-proposals/pull/1767)
extensible events rules
- `msc3381_polls_enabled`:
[MSC3381](https://github.com/matrix-org/matrix-spec-proposals/pull/3381)
poll rules
- `msc3664_enabled`:
[MSC3664](https://github.com/matrix-org/matrix-spec-proposals/pull/3664)
reply rule
- `msc4028_push_encrypted_events`:
[MSC4028](https://github.com/matrix-org/matrix-spec-proposals/pull/4028)
encrypted event rule
- `msc4306_enabled`:
[MSC4306](https://github.com/matrix-org/matrix-spec-proposals/pull/4306)
thread subscription rules
- `msc4210_enabled`: removes `.m.rule.contains_display_name`,
`.m.rule.contains_user_name` and `.m.rule.roomnotif`
`GET /pushrules/` only lists the rules that exist for that
configuration. `GET` on a single rule that is not in the set returns
`404 M_NOT_FOUND`. The same set is used to evaluate notifications, and
to build the `m.push_rules` account data in `/sync`.
`PUT .../enabled` and `PUT .../actions` did not use that set. They
accepted any rule ID that Synapse knows about, whether or not the server
currently serves it. Clients could modify a rule the server had just
said did not exist. The request returned `200`, the change was stored,
and it was then ignored. Both endpoints now return `404 M_NOT_FOUND` for
such a rule, as [the spec
requires](https://spec.matrix.org/v1.19/client-server-api/#put_matrixclientv3pushrulesglobalkindruleidenabled)
for a rule that does not exist.
Before (`msc4210_enabled: true`):
```
GET /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif
404 {"errcode": "M_NOT_FOUND", "error": "..."}
PUT /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif/enabled
{"enabled": false}
200 {}
```
After:
```
GET /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif
404 {"errcode": "M_NOT_FOUND", "error": "..."}
PUT /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif/enabled
{"enabled": false}
404 {"errcode": "M_NOT_FOUND", "error": "Unknown rule"}
```
This is a no-op for default configurations. All of these flags are off
by default, and rules that are served are still modifiable as before.
ESS deployments of MatrixRTC are going to run `lk-jwt-service` as an
application service so that it can manage delayed events. However,
setting up an application service is particularly inefficient, as
Synapse iterates through all the events of the rooms it covers since
their creation.
When an application service is configured with `url: null`, there is no
need to traverse all the events for it, so we can set up such
application services much faster without having to decide what to do for
other application services. Discussions about those are still open:
element-hq/synapse#11629 (see also matrix-org/synapse#1834).
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
Use a new, dedicated endpoint for scheduling delayed events. This should
deprecate the `delay` query parameter on the `/send` and `/state`
endpoints.
Co-authored-by: Paul Chobert <paul@chobert.fr>
Co-authored-by: Eric Eastwood <erice@element.io>
In a similar vein to #20315, this attempts to speed up Synapse when
delayed events aren't in use. Here we skip doing any work if the room
event stream has not advanced.
Motivated by work on MSC4354, because the sticky events MSC specs that
`received_ts` is used to calculate the
start of an event's stickiness. (It would therefore be convenient to
have it available as part of event metadata, so an event can expose a
cheap helper method to check if it is sticky.)
This commit adds `received_ts` into the `event.internal_metadata` struct,
alongside `stream_ordering`.
I conceptually treat it as a sibling to `stream_ordering` and populate
it in all the same places where `stream_ordering` is populated.
Because this needs `received_ts` to be added to the HTTP replication
response for event persistence,
I also add a temporary compatibility measure that keeps rolling worker
upgrades working during this
transition.
I intend to remove the compatibility code in the next release.
I have manually tested that
```
SYNAPSE_POSTGRES=1 SYNAPSE_POSTGRES_USER=postgres SYNAPSE_POSTGRES_HOST=localhost SYNAPSE_POSTGRES_PASSWORD=postgres poetry run trial -j 6 tests.replication.test_sharded_event_persister
```
fails only when both the compatibility code is removed and the
`"received_ts"` key is commented out in the response (to omit it).
The compatibility code causes the test to pass even when `"received_ts"`
is commented out.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Fixes: #19661
Part of: MSC4354
First introduced some helpers for querying a sharded stream, which were moved to
https://github.com/element-hq/synapse/pull/20108.
- SQL builder for 'give me rows between two sharded tokens'
- a function to return the 'new' from_token after a limited read
Then make the sticky events portions of both oldschool and sliding sync use
sharded tokens.
This prevents a 'torn sync' problem where, because the events and sticky
events streams are both sharded with the same writers:
1. persisting a sticky event leads to an event row and a `sticky_events`
row being created for event `e` (on writer `W`).
2. Suppose another writer is still persisting, so the position of the
stream for writer W advances, but the global
['linear'](https://element-hq.github.io/synapse/latest/development/synapse_architecture/streams.html?highlight=%E2%80%9Clinear%E2%80%9D#current-stream-id)
(I have been calling this 'baseline') position does not advance yet.
3. sync is sharding-aware for the regular events stream, so `e` appears
as a real-time timeline event, thus coming down sync
4. since sticky events were not sharding-aware, `e` is not detected as a
sticky event in this sync response. The sticky event position doesn't
advance in the sync token, either.
5. (assume the position advances because other writers have finished
writing and so the 'linear' position advances for the sticky events
stream)
6. the client then syncs again, using the new sync token. This time, `e`
appears in the sticky event section.
If we make sticky events sharding-aware in sync, then `e` would appear
for consideration in both the timeline section **and** the sticky event
section in the first sync.
It then gets deduplicated out of the sticky section since it is
appearing in the timeline section in the same sync.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Co-authored-by: Eric Eastwood <erice@element.io>
Part of: MSC4354 (Sticky Events)
Our locally-sent Sticky Events need to be reliably (within bounds of
possibility) sent to remote homeservers.
This adds the mechanism to make this happen.
Notably omits catching up historical sticky events to homeservers that only just
joined the room.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Co-authored-by: Eric Eastwood <erice@element.io>
In `/sync` there is a mode called lazy loaded (LL) members. In this mode
`/sync` does not return all memberships in the room, instead it only
returns a) membership changes, and b) the memberships of senders of
events in the timeline (if not previously sent).
Synapse implements a lazy-loaded member cache to avoid repeatedly
sending down the same memberships when a single user sends multiple
events. It does this by storing for each user/device the memberships it
has previously returned in an LRU cache. Note that it is fine for the
server to return the same membership, clients handle that fine, but it
is a waste of bandwidth.
The current lazy-loaded membership cache has three intertwined flaws:
1. It does not handle retries or clients re-using old tokens.
2. It incorrectly applies the membership cache to membership changes,
not just memberships added by lazy-loading.
3. It does not correctly handle multiple "sync streams" from the same
device.
In this PR we explicitly fix the first two issues in two commits, and
the third issue is mitigated:
1. Alongside each membership in the LL cache we store the `next_batch`
position of the response that carried it. A client that processed a
response syncs from its `next_batch` next. When a request arrives with a
`since` token, we delete every membership recorded after that token, as
they were sent in responses the client never processed. Memberships
recorded at or before `since` are kept.
2. We always return memberships that have changed, even if they appear
in the LL member cache.
These changes also half mitigate the issue of running multiple `/sync`
loops per-device (though we do not see this in the wild), as we'd at
least always return the changed membership states.
Fixes#19978 and #20278, supersedes #20279
Part of: MSC4354,
Part of: https://github.com/element-hq/synapse/issues/19641
This PR persists events in redacted form when they already have valid
redactions. That's not directly a sticky events-specific change (and I
feel it makes sense to redact immediately before persistence when we
already have the redaction).
This causes a nice effect which is that the event's stickiness is lost
immediately before persistence, because the `msc4354_sticky` field is
redacted by the redaction algorithm.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Co-authored-by: Eric Eastwood <erice@element.io>
Fixes#15871
### What was broken
Sending `{"auth": null}` to any endpoint that requires user-interactive
auth (for example `POST /keys/device_signing/upload`, `DELETE
/devices/{deviceId}` or `POST /register`) returned a 500 instead of the
usual 401 with the available flows. A non-object `auth` on endpoints
that do not validate the body with a model (such as `/register`) could
also 500, e.g. `{"auth": ["session"]}`.
### Root cause
`AuthHandler.check_ui_auth` did `clientdict.pop("auth", {})`, so the
`{}` default only applies when the key is missing. An explicit `null`
came through as `None` and the following `"session" in authdict` raised
`TypeError`. `get_session_id` had the same assumption that `auth` is a
dict.
### Pull Request Checklist
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Signed-off-by: Ankit Jha <ankit.jha@tradomate.one>
Add a metric for the number of times we see a stall due to being given a
future token.
This would have caught
https://github.com/element-hq/synapse/issues/20080, where lots of sync
streams got stuck due to a stream not getting replicated correctly.
Tokio tasks currently complete by taking the GIL and calling
`reactor.callFromThread`. Taking the GIL may block for a period of time,
and we do not want that to happen on the tokio reactor threads (as that
can block other work from happening).
To avoid this, we instead add a work queue that we can add to from Rust
without taking the GIL, which is drained by the reactor. We signal to
the Twisted reactor that it should wake up by using a unix socket pair,
which is exactly how `reactor.callFromThread` works.
---
The self-pipe trick is basically where you create a pair of unix sockets
connected to each other. One end is added to the reactor so the reactor
is woken up when there are bytes to read, and when another thread needs
to wake up the reactor it just needs to write a byte into the other unix
socket.
---------
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
This pull request fixes a few issues with the profile update stream,
when a user leaves a room. This is basically just mimicking what we
already had for when someone leaves a room, and they no longer share any
rooms, but in reverse - ie when we leave a room, and no longer share
rooms with some users. This was missed in the implementation when adding
the profile update stream for legacy and sliding sync.
[Fix missing profile update stream rows on user leaving
room](https://github.com/element-hq/synapse/commit/e953322226731a99718ebe13139b9c5f05ae4b62)
When a user leaves a room, profile update stream rows are generated with
the `LEFT_ROOM` action for each user in the room that no longer shares a
room with the user who left the room.
This also needs to happen in reverse. The user who left the room needs
to have a profile update stream row for each user they no longer share a
room with.
If we don't do this, clients may keep stale data around even after they
don't share a room with a user, which may mean they don't know when to
refetch profiles after re-joining a room with the stale profile data
user.
[Clear out old profile update stream rows when leaving a
room](https://github.com/element-hq/synapse/commit/e7642bf141b098c1eefaf74655e5e7e18415500b)
When we leave a room, ensure profile update stream rows are cleared out
for every user we no longer share a room with. This is the same as what
happens when someone else leaves a room, but in reverse. The only
remaining profile update stream row should be the `LEFT_ROOM` action.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
This is to prevent the case where URL previews return a MXC that
immediately 404s because the media in question has been quarantined.
This is mostly to help implementations which currently show an ugly
empty preview due to the MXC being sent down, despite being invalid.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [ ] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
[MSC4140](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/4140-delayed-events-futures.md),
"Account deactivation":
> when an account is deactivated, the homeserver MUST cancel that
account's delayed events which have not yet been added to a room's event
DAG. These cancelled records MAY be removed immediately, including their
stored event content, as an exception to the usual finalised-record
retention policy.
Deactivation currently leaves the user's scheduled delayed events,
including their content, in place, and they are still attempted at their
scheduled time:
| Delayed event scheduled by the user | What happens after deactivation
today |
| --- | --- |
| Message or state event, fires after deactivation has made the user
leave the room | The send fails with a 403 (user not in room) and the
record is dropped |
| Message or state event, fires before deactivation has made the user
leave the room (leaving happens room by room in the background) | The
event is sent |
| `m.room.member` join for themselves in a public room | The deactivated
user re-joins the room |
### What changes
- Deactivating an account (client or admin API) removes the user's
unsent delayed events as its first step, and re-arms the send timer for
whatever is scheduled next.
- Deactivation can run on a worker, while the send timer lives on the
main process, so the cancellation goes through a new replication
request.
- A delayed event whose send has already started is left alone, as with
a normal cancel: it is already on its way into the DAG, and the send
path removes its record itself.
Suspension and locking are unchanged.
### After #19038
Today a cancelled delayed event is simply deleted, so this PR deletes
the user's records too. #19038 changes cancellation to keep the record
and mark it as cancelled ("finalised"), so that clients can look up what
happened to a delayed event. Once it has landed, deactivation should
probably finalise the user's records as cancelled the same way, rather
than delete them. The MSC allows either: it permits removing the records
immediately, content included, as an exception to the usual retention of
finalised records, which is worth doing at least when the user asks to
be erased.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Andrew Ferrazzutti <af_0_af@hotmail.com>
Co-authored-by: Andrew Ferrazzutti <andrewf@element.io>
As I was working on https://github.com/element-hq/synapse/pull/20218, I
noticed what seemed to be an illegal configuration of synapse.
- `require_auth_for_profile_requests`: blocks profile requests unless
authenticated
- `limit_profile_requests_to_users_who_share_rooms`: blocks profile
requests unless authenticated user share a room with requested user
This, I think, should be an illegal config:
```
require_auth_for_profile_requests = false
limit_profile_requests_to_users_who_share_rooms = true
```
As of now, with such a config the shared-room check is never applied: a
profile can be requested anonymously, and also by an authenticated user
who doesn't share a room.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Olivier 'reivilibre' <oliverw@element.io>
The count of unread notifications is computed in two phases:
1. From the summaries (`event_push_summary`) when those are up to date
with the user's last read receipt
2. Otherwise from counting the push actions (`event_push_actions`) after
that receipt
The problem in this process is that the threads whose summary is up to
date are identified with only their `thread_id`. But all main timelines
share the same `"main"` thread_id, so as soon as one room has an
up-to-date summary, phase 2 skips the main timeline of every room.
The fix identifies the up-to-date summaries with their `(room_id,
thread_id)` pair.
Authored by @sandhose. Found while investigating the
`TestThreadedReceipts` Complement flake (#15517, #18537), but it is a
bug on its own.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Quentin Gliech <quentingliech@gmail.com>
Co-authored-by: Devon Hudson <devonhudson@librem.one>
We sometimes see a lot of `ERROR` logs like the following:
```
Closing scope Scope<... master.write_bytes_to_request> which is not the currently-active one None
```
This error is generated by `opentracing.Scope` checking that it is the
"active" one. Synapse tracks "active" spans via logcontexts, so this
indirectly asserts that the scope is closed in the context it was opened
in. However, the producer methods are often called from the reactor and
therefore withing the sentinel logcontext, which produces the error
above.
This specifically happens when the producer tries to write large
responses but gets paused, and then later resumes.
The fix is to simply use `Span` directly, rather than scopes. `Span`
does not perform the checks.
I noticed this when deploying #19979, though it is unrelated.
### Summary
Resolves#18308.
Makes the `TaskScheduler`'s maximum concurrent running tasks
configurable via `task_scheduler.max_concurrent_tasks` in the homeserver
configuration and defaults it to `2` (reduced from the previous
hardcoded limit of `5`).
### Motivation
Twisted's `adbapi.ConnectionPool` defaults to 5 database connections
(`cp_max=5`). When the `TaskScheduler` previously executed up to 5 tasks
concurrently, it could exhaust the entire database connection pool,
starving regular API and synchronization requests on smaller homeserver
instances. Lowering the default to 2 preserves at least 3 connections
for foreground requests while allowing concurrent tasks to progress, and
gives administrators the ability to tune the limit according to their
host and connection pool sizing.
### Changes
- Added `TaskSchedulerConfig` (`task_scheduler.max_concurrent_tasks`)
with validation (must be a positive integer) and registered it in
`HomeServerConfig`.
- Updated `TaskScheduler` to enforce the configured concurrency limit
and updated class default to 2.
- Updated config documentation and JSON schema.
- Added config tests in `tests/config/test_task_scheduler_config.py` and
updated unit tests in `tests/util/test_task_scheduler.py`.
- Added newsfragment in `changelog.d/18308.feature`.
---------
Signed-off-by: nevil06 <nevilansondsouza@gmail.com>
Co-authored-by: Devon Hudson <devonhudson@librem.one>
When a stream advances in the database but stops being replicated to a
process, that process's view of the stream freezes. Requests that wait
for it to catch up to a token issued by another worker then time out and
return empty responses indefinitely (see #20080), and nothing exported
said so.
Report `get_current_token` from every ID generator, on every process.
The value is comparable between processes, so a stream that has stopped
reaching one of them shows up as divergence with no client traffic
needed. It is also the position that `wait_for_stream_token` waits on,
so its divergence is the failure itself rather than a proxy for it.
Being a watermark over gapless runs of persisted IDs, it also catches a
single writer of a sharded stream going quiet, which a maximum across
writers would hide behind the writers still being replicated.
Previously the tokio runtime was stashed in a hidden attribute on the
reactor object, installed lazily by whichever Rust code first needed it,
and started via `callWhenRunning`.
Instead, we create a `RustRuntime` (accessible via
`HomeServer.get_rust_runtime()`) that holds any per-reactor Rust state,
such as the tokio runtime. It is constructed lazily on use. Rust
consumers (`HttpClient`, `VersionsHandler`, the Python DB pool wrapper)
now receive the runtime or reactor handle explicitly, and the
`reactor.run()` / manual-startup workarounds in tests are no longer
needed.
We also add helper wrappers in Rust for `Reactor` and `HomeServer` that
exposes the needed functionality.
The aim is to allow us to have a Rust-side clock (mainly to get the
current time), that respects the unit test per-reactor time management.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This is an attempt to better cache the cases where there are a large
number of extremities to resolve over, which keep slightly changing.
This spawns from seeing issues on matrix.org.
We already have a cache over the exact state groups being resolved.
However, we can do better by caching the inputs into state res (i.e. the
conflicted sets), which are more likely to be constant across repeated
state res in a room. We key this cache based on a sha256 hash, on the
assumption that this will never conflict.
Also includes a commit that removes needless copying of the state.
Ports the logcontext classes to Rust, and gives tokio tasks a captured
logcontext so that work running in (or spawned from) Rust is attributed
to the request that caused it.
1. **Add characterization tests for logcontext error messages and the
filter** — pins the exact `logcontext_error` message shapes, the
abuse-detection code paths and `LoggingContextFilter`'s observable
behaviour, *against the existing Python implementation* (this commit is
green on its own). These are the behavioural contract the port has to
satisfy.
2. **Port `ContextResourceUsage` to a Rust pyclass** — self-contained:
the new `synapse_rust.logcontext` module, the class, its stub and the
re-export.
3. **Move the logcontext storage and `LoggingContext` to Rust** — the
core change; the commit message carries detailed design notes.
Highlights:
- The slot is typed `Option<Py<LoggingContext>>`, with `None`
representing the sentinel. `_Sentinel`/`SENTINEL_CONTEXT` stay pure
Python (unchanged); thin wrappers on
`current_context`/`set_current_context` convert at the boundary, and
pyo3's extraction enforces the type (`TypeError` otherwise).
- The accounting is native: one `getrusage(RUSAGE_THREAD)` read per
switch via libc, inline `stop`/`start` bookkeeping for base
`LoggingContext`s, Python dispatch only for subclasses
(`BackgroundProcessLoggingContext`) so their overrides run. The thread
id comes from `PyThread_get_thread_ident` (the exact
`threading.get_ident()`
value) without calling into Python.
- The hot paths avoid per-operation allocation: names are `Py<PyString>`
(the per-log-record `str(context)`/`server_name` reads are INCREF-only),
error branches materialise strings only when hit.
4. **Attribute Rust-spawned work to the caller's logcontext** —
`create_deferred` captures the caller's context and scopes it onto the
spawned task via a tokio task-local (`LogContextHandle`);
`current_context()` gives the task-local read precedence, so
`LoggingContextFilter`/`pyo3-log` resolve the right context on worker
threads with no per-record stamping. `run_python_awaitable` restores the
captured context (via a `with_logcontext` helper, the Rust
`PreserveLoggingContext`) around Python called back from Rust, so e.g.
`runInteraction` from the Rust `/versions` handler accounts its DB usage
against the right request. Integration tests exercise both guarantees
through real production code paths.
Follow-up work on top of this (separate PR): porting
`BackgroundProcessLoggingContext` natively and removing further `Py<_>`
indirections. The fact that `BackgroundProcessLoggingContext` is a
subclass is what forces some of the warts in this PR: e.g. having to use
`Py<LoggingContext>` everywhere, etc.
We don't try (yet) to make this pure Rust, instead we see this as simply
maintaining the Python logcontext machinery when crossing, rather than
trying to make a Rust equivalent that can be used by pure Rust
dependencies. We probably do want to do that in future, as well as wire
up e.g. CPU recording on Rust side, but that is unnecessary for now.
For state filters that ask for concrete types. This allows us to cache
the common case of asking for a specific type/state key.
I noticed a bunch of queries in the jaeger traces that could be cached.