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

Fix reloading in Ruby 3.3 #730

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thomasmarshall
Copy link

This commit ensures the application is restarted when a preloaded file is changed. This has been silently broken because the fallback behavior still results in the application being restarted, but only when the user next runs a command—hence the acceptance test still passing.

The behavior of BasicSocket#recv changed in Ruby 3.3 such that it now returns nil on a closed stream socket, instead of an empty string. When we call #empty? on nil the NoMethodError is swallowed by the failsafe thread and the application is not restarted.

This commit fixes the issue by checking both #nil? and #empty? so it works with both old and new versions. It also updates the acceptance test to be more specific about what it considers a "reload" by asserting the relevant log line exists.

This commit ensures the application is restarted when a preloaded file
is changed. This has been silently broken because the fallback behavior
still results in the application being restarted, but only when the user
next runs a command—hence the acceptance test still passing.

The behavior of `BasicSocket#recv` changed in Ruby 3.3 [1] such that it
now returns `nil` on a closed stream socket, instead of an empty string.
When we call `#empty?` on `nil` the `NoMethodError` is swallowed by the
failsafe thread and the application is not restarted.

This commit fixes the issue by checking both `#nil?` and `#empty?` so it
works with both old and new versions. It also updates the acceptance
test to be more specific about what it considers a "reload" by
asserting the relevant log line exists.

[1]: ruby/ruby#6407
@thomasmarshall
Copy link
Author

These logs were produced with a Rails app using Ruby 3.3.5.

  • bin/spring server
  • bin/rails test test/some_test.rb
  • touch config/application.rb

Before

[2024-10-01 17:19:48 +0100] [41338] [watcher:test] check_stale: mtime=1727785028.0366342 < computed=1727799587.8757818
[2024-10-01 17:19:48 +0100] [41338] [watcher:test] marked stale, calling listeners: listeners=[…]
[2024-10-01 17:19:48 +0100] [41338] [application:test] running -> watcher_stale
[2024-10-01 17:19:48 +0100] [41338] [application:test] watcher_stale -> exiting

After

[2024-10-01 17:20:55 +0100] [41375] [watcher:test] check_stale: mtime=1727799587.8757818 < computed=1727799654.9323258
[2024-10-01 17:20:55 +0100] [41375] [watcher:test] marked stale, calling listeners: listeners=[…]
[2024-10-01 17:20:55 +0100] [41375] [application:test] running -> watcher_stale
[2024-10-01 17:20:55 +0100] [41375] [application:test] watcher_stale -> exiting
[2024-10-01 17:20:55 +0100] [41373] [application_manager:test] child 41375 shutdown
[2024-10-01 17:20:55 +0100] [41381] [application:test] preloading app
[2024-10-01 17:20:55 +0100] [41381] [watcher:test] start: poller=nil
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] watcher: add: […]
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] subjects_changed: mtime 0 -> 1721481941.5332227
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] watcher: add: […]
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] subjects_changed: mtime 1721481941.5332227 -> 1727799654.9323258
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] watcher: add: […]
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] subjects_changed: mtime 1727799654.9323258 -> 1727799654.9323258
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] watcher: add: […]
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] subjects_changed: mtime 1727799654.9323258 -> 1727799654.9323258
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] watcher: add: […]
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] subjects_changed: mtime 1727799654.9323258 -> 1727799654.9323258
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] watcher: add: […]
[2024-10-01 17:20:56 +0100] [41381] [watcher:test] subjects_changed: mtime 1727799654.9323258 -> 1727799654.9323258
[2024-10-01 17:20:56 +0100] [41381] [application:test] initialized -> running

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 this pull request may close these issues.

1 participant