Skip to content

Add warning about known problems when configuring use_frozen_dicts #19711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MadLittleMods merged 5 commits into from
Apr 24, 2026
+14 −1
Merged

Add warning about known problems when configuring use_frozen_dicts #19711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
60f227f
Add warning about trying to configure `use_frozen_dicts`
MadLittleMods Apr 20, 2026
2ec83ba
Add changelog
MadLittleMods Apr 20, 2026
15dfb80
Generate docs
MadLittleMods Apr 20, 2026
2003339
Add extra new line (align with other spots)
MadLittleMods Apr 20, 2026
9827f37
Add note in the code as well
MadLittleMods Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/19711.doc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add warning about known problems when configuring `use_frozen_dicts`.
6 changes: 5 additions & 1 deletion docs/usage/configuration/config_documentation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,11 @@ user_agent_suffix: ' (I''m a teapot; Linux x86_64)'
---
### `use_frozen_dicts`

*(boolean)* Determines whether we should freeze the internal dict object in `FrozenEvent`. Freezing prevents bugs where we accidentally share e.g. signature dicts. However, freezing a dict is expensive. Defaults to `false`.
*(boolean)* Determines whether we should freeze the internal dict object in `FrozenEvent`. Freezing prevents bugs where we accidentally share e.g. signature dicts. However, freezing a dict is expensive.

> ⚠️ **Warning** – This option is known to introduce a new class of [comparison bugs](https://github.com/element-hq/synapse/issues/18117) in Synapse.
Copy link
Copy Markdown
Contributor

@frebib frebib Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this implementation is buggy, wouldn't it just be better to remove it, or at least disabled it unconditionally until it works? Seems ill advised to me to leave options in that actively break software

Copy link
Copy Markdown
Contributor Author

@MadLittleMods MadLittleMods Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed 👍 PR description suggests a follow-up to remove.

I'd rather document this now than be held up by a refactor that may never happen or is a more controversial change in terms of getting it across the line. It also makes the follow-up PR more clear as the reasoning is contained within.


Defaults to `false`.

Example configuration:
```yaml
Expand Down
4 changes: 4 additions & 0 deletions schema/synapse-config.schema.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ properties:
Determines whether we should freeze the internal dict object in
`FrozenEvent`. Freezing prevents bugs where we accidentally share e.g.
signature dicts. However, freezing a dict is expensive.


> ⚠️ **Warning** – This option is known to introduce a new class of [comparison
bugs](https://github.com/element-hq/synapse/issues/18117) in Synapse.
Comment on lines +99 to +100
Copy link
Copy Markdown
Contributor Author

@MadLittleMods MadLittleMods Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same warning format as other places (we're not that consistent though)

synapse/schema/synapse-config.schema.yaml

Lines 3084 to 3086 in a9361c4

> ⚠️ **Warning** – The additional consequences of replacing
[`macaroon_secret_key`](#macaroon_secret_key) will apply in case it
delegates to `registration_shared_secret`.

synapse/schema/synapse-config.schema.yaml

Lines 3682 to 3686 in a9361c4

> ⚠️ **Warning** – Replacing an existing `macaroon_secret_key` with a new
one will lead to invalidation of access tokens for all guest users. It
will also break unsubscribe links in emails sent before the change. An
unlucky user might encounter a broken SSO login flow and would have to
start again.

default: false
examples:
- true
Expand Down
4 changes: 4 additions & 0 deletions synapse/events/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@
NOTE: This is overridden by the configuration by the Synapse worker apps, but
for the sake of tests, it is set here because it cannot be configured on the
homeserver object itself.

FIXME: Because of how this option works (changing the underlying types), it causes
subtle downstream bugs that makes type comparisons brittle, tracked by
https://github.com/element-hq/synapse/issues/18117
"""

T = TypeVar("T")
Expand Down
Loading
⚡