element-hq / element-hq/synapse

`LaterGauge` callbacks aren't thread safe

Open
#18,764 1 comment 0 reactions 0 assignees View on GitHub
A-Metrics O-Frequent S-Tolerable T-Defect
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

*Spawning from @reivilibre [spotting](https://matrix.to/#/!SGNQGPGUwtcPBUotTL:matrix.org/$BNSDCESfST58eDiwQXI37Ab0uvmuWZHOgNr6HnvKTDY?via=jki.re&via=matrix.org&via=element.io) this [error in Sentry](https://sentry.tools.element.io/organizations/element/issues/11209742),*

Sentry error: https://sentry.tools.element.io/organizations/element/issues/11209742
```
RuntimeError: dictionary changed size during iteration
synapse/federation/sender/__init__.py in at line 419
synapse/metrics/__init__.py in collect at line 176
```

Relevant code:

https://github.com/element-hq/synapse/blob/e16fbdcdcc95d626f6d3dcd0b6a85fdb0a06456a/synapse/federation/sender/__init__.py#L414-L423

But this also happens with all of our `LaterGauge`:

- https://github.com/element-hq/synapse/issues/12079
- https://github.com/element-hq/synapse/issues/12078
- https://github.com/element-hq/synapse/issues/14319

### Potential suspects

While this code has changed recently in https://github.com/element-hq/synapse/pull/18714, I don't see why those changes would cause this error. In fact, we see this kind of problem occuring before those changes:

- https://sentry.tools.element.io/organizations/element/issues/11209713
- https://sentry.tools.element.io/organizations/element/issues/11209699
- https://sentry.tools.element.io/organizations/element/issues/11196677
- https://sentry.tools.element.io/organizations/element/issues/11196747

I'm guessing the actual problem occurs when using the dedicated `metrics` listener type which runs on a different thread. So when we `collect` from the other thread, sometimes we collide with the dictionary changing size on the main thread.

### Potential solutions

We probably need to use `threading.Lock()` like we do for [`InFlightGauge`](https://github.com/element-hq/synapse/blob/e16fbdcdcc95d626f6d3dcd0b6a85fdb0a06456a/synapse/metrics/__init__.py#L209C7-L350)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.