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

[bex] Potential Memory Leak in Bridge class due to Unused Event Listeners #16083

Open
esindger opened this issue Jul 17, 2023 · 0 comments
Open
Labels
bug/1-hard-to-reproduce A reproduction is available, but it's hard to reproduce, so it has a lower priority. bug/1-repro-available A reproduction is available and needs to be confirmed. flavour/quasar-cli-vite kind/bug 🐞 mode/bex Qv2 🔝 Quasar v2 issues

Comments

@esindger
Copy link

esindger commented Jul 17, 2023

What happened?

The Bridge class seems to have a potential memory leak due to unused event listeners. This behavior stems from the manner in which the respond function is implemented within the on method.

In the current implementation, every time a response is sent back using the respond method within the callback function of an event, a new listener is created for the response. This listener is designed to listen for any messages coming back from the event. Here's an example usage scenario from the official documentation:

bridge.on('some.event', ({ data, respond }) => {
  console.log('Event received, responding...')
  respond(data.someKey + ' hey!')
})

In this example, respond is invoked to send back a response, and this triggers the creation of a new event listener in the _nextSend method. However, this newly created listener does not seem to be removed or reused later on, potentially leading to memory leaks if a large number of responses are sent.

What did you expect to happen?

When using the respond method within the callback function of an event, I expected the Bridge class to manage event listeners efficiently. Specifically, I expected that any event listener created for the response would be removed or reused after the response is handled, to avoid any potential memory leaks.

Potential solution:

A possible solution to this issue could be to modify the respond method to flag responses, and then adjust the _nextSend method to not add new listeners for responses.

Please review the following proposed changes:

In the on method:

respond: (payload /* optional */) => this._send([{ event: originalPayload.eventResponseKey, payload, isResponse: true }])

In the _nextSend method:

  if(!currentMessage.isResponse) {
      let allChunks = []


      const fn = (r) => {
        //...
      }

      this.on(eventResponseKey, fn)
  }

Reproduction URL

https://github.com/quasarframework/quasar/blob/0302bb3ccee01a7e0cdd748fa1155bb6841565e9/app-vite/templates/entry/bex/bridge.js

How to reproduce?

  1. Listen to an event using the on method.
  2. Within the callback function for the event, use the provided respond method.
  3. Observe that new listeners are generated but never removed.

Flavour

Quasar CLI with Vite (@quasar/cli | @quasar/app-vite)

Areas

BEX Mode

Platforms/Browsers

Chrome

Quasar info output

No response

Relevant log output

No response

Additional context

No response

@esindger esindger added kind/bug 🐞 Qv2 🔝 Quasar v2 issues labels Jul 17, 2023
@github-actions github-actions bot added bug/1-hard-to-reproduce A reproduction is available, but it's hard to reproduce, so it has a lower priority. bug/1-repro-available A reproduction is available and needs to be confirmed. flavour/quasar-cli-vite mode/bex labels Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/1-hard-to-reproduce A reproduction is available, but it's hard to reproduce, so it has a lower priority. bug/1-repro-available A reproduction is available and needs to be confirmed. flavour/quasar-cli-vite kind/bug 🐞 mode/bex Qv2 🔝 Quasar v2 issues
Projects
None yet
Development

No branches or pull requests

1 participant