matrix-org / matrix-org/matrix-js-sdk

MaxListenersExceededWarning on threads; many redundant fetches of thread root event

Open
#3,463 9 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
2.2k
Forks
704
Avg merge
1d 20h
Merged PRs (30d)
40

Description

Using an account that has some threads, I get the following warning around the first sync:

```
MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 Thread.update listeners added. Use emitter.setMaxListeners() to increase limit
```

Stack trace

```
MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 Thread.update listeners added. Use emitter.setMaxListeners() to increase limit
_addListener events.js:211
addListener events.js:227
on typed-event-emitter.ts:133
reEmit ReEmitter.ts:57
reEmit ReEmitter.ts:84
setThread event.ts:1618
mapper event-mapper.ts:74
fetchRootEvent thread.ts:148
updateThreadMetadata thread.ts:482
addEvents thread.ts:291
createThread room.ts:2277
addThreadedEvents room.ts:2183
addLiveEvents room.ts:2843
addLiveEvents room.ts:2842
injectRoomEvents sync.ts:1779
processSyncResponse sync.ts:1437
promiseMapSeries utils.ts:425
processSyncResponse sync.ts:1293
doSync sync.ts:915
sync sync.ts:798
startClient client.ts:1513
initializeMatrixConnection App.tsx:39
App App.tsx:46
```

I haven't encountered any user-impacting issues around this, ~but it sounds like this could cause `MatrixEvent`s to fail to emit `Thread.update` to certain listeners.~ It looks like this is simply a warning and does not prevent listeners from being added: https://github.com/browserify/events/blob/48e3d18659caf72d94d319871106f089bb40002d/events.js#L211 . This is still noise that could mask a real, easily-identifiable memory leak.

**More importantly,** I believe this is caused by a bug in `Thread#fetchRootEvent` where the remote root event is fetched on every call to `updateThreadMetadata` (e.g. when adding an event to the thread, as will be done for each thread event received during a sync):

https://github.com/matrix-org/matrix-js-sdk/blob/dfb079a76fcdf2c218e8b6ba08d4d8f5f85fa10c/src/models/thread.ts#L142-L153

Note that this code both checks the cache for an event, then seems to indiscriminately fetch the root event from remote. According to the stack trace, this remote fetch is what leads to the redundant event subscriptions. (If I look at the network tab, I can see that there are many calls to `/rooms/{roomId}/event/{eventId}` for the same event, which I think is the thread root.)

I can't say for sure that this is the cause, but it at least seems like a bug in itself. If these are not related issues, I'd be happy to split this issue.

If this is a correct diagnostic, you should be able to reproduce the issue using [this repo](https://bitbucket.org/davidisaaclee/matrix-js-sdk-minimal-example/src/thread-fetch-root-event/) on an account that has a thread with at least 11 replies (11 = EventEmitter max listeners).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/models/thread.ts around Thread#fetchRootEvent at lines 142-153, then trace updateThreadMetadata and the setThread/ReEmitter calls shown in the stack trace. Reproduce with the linked minimal example using a thread with at least 11 replies and inspect repeated root-event requests. Done means establishing the cause and eliminating the redundant fetches or confirming the separate warning is addressed.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.