matrix-org / matrix-org/matrix-js-sdk
MaxListenersExceededWarning on threads; many redundant fetches of thread root event
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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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