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

Warning in Ruby 3.0 #113

Closed
thomasklemm opened this issue Feb 18, 2021 · 6 comments · Fixed by #116
Closed

Warning in Ruby 3.0 #113

thomasklemm opened this issue Feb 18, 2021 · 6 comments · Fixed by #116

Comments

@thomasklemm
Copy link

thomasklemm commented Feb 18, 2021

After upgrading to Ruby 3.0, the following warning gets printed when running system specs right before the first reflex invocation in a spec example:

.../ruby/gems/3.0.0/gems/cable_ready-4.5.0/lib/cable_ready/channel.rb:14: 
warning: finalizer references object to be finalized

Link to the line in question.

ObjectSpace.define_finalizer self, -> { CableReady.config.delete_observer config_observer }
@marcoroth
Copy link
Member

marcoroth commented Feb 20, 2021

Hey @thomasklemm, thank you for reporting this issue!

Do you have a clue how that should look like in Ruby 3?

To be honest I don't fully grok what needs to be changed in order to be ready for Ruby 3.

The code was introduced in #96. Maybe @hopsoft can expand on it.

@thomasklemm
Copy link
Author

thomasklemm commented Feb 22, 2021

It looks like the warning gets raised since self is being passed to ObjectSpace.define_finalizer, according to the warning spec'd here. Not sure what's best for Ruby 3.0.

Edit:
The warning came to be through this discussion and PR: Feature #15794: Warn in verbose mode on defining a finalizer that captures the object

Edit 2:
Actually this seems to be the last PR bringing the warning in: ruby/ruby#3444

Looks like the proc holding a reference to config_observer (which self is assigned to`) could be the issue. Not really sure though what to do.

@n-rodriguez
Copy link
Contributor

This might be a solution : https://www.mikeperham.com/2010/02/24/the-trouble-with-ruby-finalizers/

@leastbad
Copy link
Contributor

leastbad commented Mar 7, 2021

@thomasklemm could you try out the https://github.com/hopsoft/cable_ready/tree/hopsoft/finalize branch from #116 and confirm that it addresses the issues you've seen?

@alexander-makarenko
Copy link

@leastbad Indeed, #116 solves the problem.

@thomasklemm
Copy link
Author

@leastbad #116 works, the warnings are gone when using the PR branch 👍

hopsoft added a commit that referenced this issue Mar 8, 2021
* Setup better finalizer

SEE: https://www.mikeperham.com/2010/02/24/the-trouble-with-ruby-finalizers/

Resolves #113

* Add some defensive programming
gl3n91 pushed a commit to gl3n91/cable_ready that referenced this issue Apr 12, 2022
woahdae pushed a commit to InPlay/cable_ready that referenced this issue Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants