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

Diagnostics channels get garbage collected even when used #42170

Closed
rochdev opened this issue Mar 1, 2022 · 1 comment
Closed

Diagnostics channels get garbage collected even when used #42170

rochdev opened this issue Mar 1, 2022 · 1 comment
Labels
diagnostics_channel Issues and PRs related to diagnostics channel

Comments

@rochdev
Copy link
Contributor

rochdev commented Mar 1, 2022

Version

17.6.0

Platform

No response

Subsystem

No response

What steps will reproduce the bug?

node --expose-gc test.js
const { channel } = require('diagnostics_channel')
const strongRef = channel('strong')

strongRef.subscribe(message => {
  console.log(message) // outputs because the subscriber is still available
})

channel('weak').subscribe(message => {
  console.log(message) // no output because the subscriber was garbage collected
})

setTimeout(() => {
  channel('weak').publish('weak output')
  strongRef.publish('strong output')
})

gc()

How often does it reproduce? Is there a required condition?

Whenever a channel is used multiple times without a strong reference to the returned object and with any garbage collection happening between uses.

What is the expected behavior?

It should be possible to reference channels by name and expect them to be retained without explicitly holding a reference to the returned value of the function.

What do you see instead?

Channels are seemingly randomly removed whenever garbage collection happens if there is no strong reference to the returned channel.

Additional information

Every single member in my team got bit by this at one point or another. This is also true for external users trying to contribute to our repository or to other libraries, for example DataDog/dd-trace-js#1642 (comment).

My understanding is this was done to allow dynamic channel names that would later be garbage collected. Unfortunately, this resulted in this bug. Since dynamic channel names is an edge case, I think the priority should be on fixing the bug, and if anyone actually needs dynamic channel names then there could be a method added to cleanup channels by name, or to explicitly make them weak references.

cc @Qard since he did the original implementation.

@legendecas
Copy link
Member

legendecas commented Mar 5, 2024

This has been fixed by #44943 since the channels increase ref counting when there are active subscribers in that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagnostics_channel Issues and PRs related to diagnostics channel
Projects
None yet
Development

No branches or pull requests

3 participants