Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Memory leak in /sync when opentracing is enabled #13282

Closed
squahtx opened this issue Jul 15, 2022 · 3 comments · Fixed by #13352
Closed

Memory leak in /sync when opentracing is enabled #13282

squahtx opened this issue Jul 15, 2022 · 3 comments · Fixed by #13352
Labels
A-Sync defects related to /sync S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@squahtx
Copy link
Contributor

squahtx commented Jul 15, 2022

The latest version of frozendict, 2.3.2, has a memory leak in __repr__ which gets triggered during /sync when opentracing is enabled.

To demonstrate:

d = {"asdsd": "dklfj"}
while True:
    _ = repr(frozendict(d))

__repr__ gets hit during /sync like so (line numbers are wonky for some reason):

Thread 1 "python" hit Breakpoint 1, frozendict_repr (mp=0x7f62eada3130) at frozendict/src/3_8/frozendictobject.c:300
300     in frozendict/src/3_8/frozendictobject.c
(gdb) py-bt
Traceback (most recent call first):
  File "<attrs generated repr synapse.types.RoomStreamToken>", line 13, in __repr__
  File "<attrs generated repr synapse.types.StreamToken>", line 13, in __repr__
  File "/home/squah/repos/synapse/.venv/lib/python3.8/site-packages/jaeger_client/thrift.py", line 58, in _to_string
    return str(s)
  File "/home/squah/repos/synapse/.venv/lib/python3.8/site-packages/jaeger_client/thrift.py", line 107, in _make_string_tag
    value = _to_string(value)
  File "/home/squah/repos/synapse/.venv/lib/python3.8/site-packages/jaeger_client/thrift.py", line 1366, in make_tag
  File "/home/squah/repos/synapse/.venv/lib/python3.8/site-packages/jaeger_client/thrift.py", line 669, in <listcomp>
  File "/home/squah/repos/synapse/.venv/lib/python3.8/site-packages/jaeger_client/thrift.py", line 412, in make_tags
  File "/home/squah/repos/synapse/.venv/lib/python3.8/site-packages/jaeger_client/thrift.py", line 422, in make_log
  File "/home/squah/repos/synapse/.venv/lib/python3.8/site-packages/jaeger_client/span.py", line 397, in log_kv
  File "/home/squah/repos/synapse/synapse/logging/opentracing.py", line 614, in log_kv
    opentracing.tracer.active_span.log_kv(key_values, timestamp)
  File "/home/squah/repos/synapse/synapse/logging/opentracing.py", line 632, in ensure_active_span_inner_2
    """
  File "/home/squah/repos/synapse/synapse/handlers/sync.py", line 2771, in _generate_room_entry
  File "/home/squah/repos/synapse/synapse/handlers/sync.py", line 1818, in handle_room_entries
    # We loop through all room ids, even if there are no new events, in case
  File "/home/squah/repos/synapse/synapse/util/async_helpers.py", line 227, in _concurrently_execute_inner
    await maybe_awaitable(func(value))

Once the memory leak in frozendict has been fixed, we need to update our locked dependencies.
This might not be the only memory leak during sync, since memory usage still appears to grow with lots of initial /sync requests even with opentracing disabled.

@squahtx squahtx added A-Sync defects related to /sync S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Jul 15, 2022
@DMRobertson
Copy link
Contributor

Cross-ref: Marco-Sulla/python-frozendict#59

@squahtx
Copy link
Contributor Author

squahtx commented Jul 15, 2022

Might contribute to #12160, along with #13286.

@squahtx squahtx added this to the Revisit: Next Month milestone Jul 15, 2022
@clokep
Copy link
Member

clokep commented Jul 21, 2022

Looks like frozendict v2.3.3 contains the fix. 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Sync defects related to /sync S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants