Skip to content
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.

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

Possible EventEmitter memory leak detected. 11 RoomMember.membership listeners added. #10456

Closed
jryans opened this issue Jul 30, 2019 · 11 comments · Fixed by matrix-org/matrix-react-sdk#3260
Assignees

Comments

@jryans
Copy link
Collaborator

jryans commented Jul 30, 2019

This is due to recent changes from matrix-org/matrix-react-sdk#3255.

It's not a leak in this case, but we are adding lots of listeners since every message now listens to the room for permissions changes, even though the user's permission state should be the same throughout the room.

I would suggest computing this permission info higher up so it happens once per room and passed down as props. This should help us leverage React's update cycle more naturally as well (it should update all components in one go, rather than each message updating separately due to having its own listeners).

@jryans
Copy link
Collaborator Author

jryans commented Jul 30, 2019

MaxListenersExceededWarning: "Possible EventEmitter memory leak detected. 11 RoomMember.membership listeners added. Use emitter.setMaxListeners() to increase limit"
    _addListener events.js:208
    addListener bundle.js:349203
    componentDidMount MessageActionBar.js:56
    React 14
    enqueueUpdate bundle.js:378946
    enqueueSetState bundle.js:379140
    setState React
    onMessageListFillRequest TimelinePanel.js:388
    _execute bluebird.js:987
    _resolveFromExecutor bluebird.js:3275
    Promise bluebird.js:2866
    onMessageListFillRequest TimelinePanel.js:389
    tryCatcher bluebird.js:5290
    _settlePromiseFromHandler bluebird.js:3302
    _settlePromise bluebird.js:3359
    _settlePromise0 bluebird.js:3404
    _settlePromises bluebird.js:3484
    _drainQueueStep bluebird.js:190
    _drainQueue bluebird.js:183
    _drainQueues bluebird.js:199
    drainQueues bluebird.js:69

@jryans
Copy link
Collaborator Author

jryans commented Jul 30, 2019

@t3chguy Are you interested in working on this?

@t3chguy
Copy link
Member

t3chguy commented Jul 30, 2019 via email

@t3chguy
Copy link
Member

t3chguy commented Jul 30, 2019

This is super annoying to wire up, needs to be passed: RoomView > TimelinePanel > MessagePanel > EventTile > MessageActionBar

@t3chguy
Copy link
Member

t3chguy commented Jul 30, 2019

I'm thinking a RoomContext which has all the useful calculated bits from RoomView would be super-useful

@turt2live
Copy link
Member

A RoomContext sounds like the perfect answer for this, I think

@t3chguy
Copy link
Member

t3chguy commented Jul 30, 2019

How stable are Contexts in React 15.6 though? I've only been using 16.8 recently

@turt2live
Copy link
Member

We've had a matrix client context for the history of the project powering a good third of the app, so I'd hope stable enough. We also want to upgrade to React 16 any day now.

@t3chguy
Copy link
Member

t3chguy commented Jul 30, 2019

We also want to upgrade to React 16 any day now.

<3

is that 16 or 16.8 (hooks)?

@turt2live
Copy link
Member

it'll probably be "whatever is latest" at the time of upgrade, depending on who does it. Ideally we'd keep up with react releases...

@t3chguy
Copy link
Member

t3chguy commented Jul 30, 2019

very exciting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants