Once again Ruby seems ready to introduce more incompatibilities and force busywork upon maintainers[1]. In order to avoid incompatibilities in the future, I used a Perl script[2] to prepend `frozen_string_literal: false' to every Ruby file. Somebody interested will have to go through every Ruby source file and enable frozen_string_literal once they've thoroughly verified it's safe to do so. [1] https://bugs.ruby-lang.org/issues/20205 [2] https://yhbt.net/add-fsl.git/74d7689/s/?b=add-fsl.perl --- v2: using add-fsl.perl instead of ed script to handle more cases: `encoding:' must precede frozen_string_literal; add after shebang and embedded Rackup CLI switches. idempotency. perl is likely installed on more systems than ed(1) nowadays even if ed is POSIX. It's certainly true for Debian: https://qa.debian.org/popcon.php?package=ed https://qa.debian.org/popcon.php?package=perl examples/linux-listener-stats.rb | 1 + examples/middleware.ru | 1 + examples/watcher.ru | 1 + examples/watcher_demo.ru | 1 + examples/yahns.conf.rb | 1 + examples/zbatery.conf.rb | 1 + ext/raindrops/extconf.rb | 1 + lib/raindrops.rb | 1 + lib/raindrops/aggregate.rb | 1 + lib/raindrops/aggregate/last_data_recv.rb | 1 + lib/raindrops/aggregate/pmq.rb | 1 + lib/raindrops/last_data_recv.rb | 1 + lib/raindrops/linux.rb | 1 + lib/raindrops/middleware.rb | 1 + lib/raindrops/middleware/proxy.rb | 1 + lib/raindrops/struct.rb | 1 + lib/raindrops/watcher.rb | 1 + setup.rb | 1 + test/ipv6_enabled.rb | 1 + test/rack_unicorn.rb | 1 + test/test_aggregate_pmq.rb | 1 + test/test_inet_diag_socket.rb | 1 + test/test_last_data_recv.rb | 1 + test/test_last_data_recv_unicorn.rb | 1 + test/test_linux.rb | 1 + test/test_linux_all_tcp_listen_stats.rb | 1 + test/test_linux_all_tcp_listen_stats_leak.rb | 1 + test/test_linux_ipv6.rb | 1 + test/test_linux_middleware.rb | 1 + test/test_linux_reuseport_tcp_listen_stats.rb | 1 + test/test_middleware.rb | 1 + test/test_middleware_unicorn.rb | 1 + test/test_middleware_unicorn_ipv6.rb | 1 + test/test_raindrops.rb | 1 + test/test_raindrops_gc.rb | 1 + test/test_struct.rb | 1 + test/test_tcp_info.rb | 1 + test/test_watcher.rb | 1 + 38 files changed, 38 insertions(+) diff --git a/examples/linux-listener-stats.rb b/examples/linux-listener-stats.rb index 7e767da..5f67633 100755 --- a/examples/linux-listener-stats.rb +++ b/examples/linux-listener-stats.rb @@ -2,0 +3 @@ +# frozen_string_literal: false diff --git a/examples/middleware.ru b/examples/middleware.ru index 642016b..a485592 100644 --- a/examples/middleware.ru +++ b/examples/middleware.ru @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/examples/watcher.ru b/examples/watcher.ru index a3e7fdb..e2aa97c 100644 --- a/examples/watcher.ru +++ b/examples/watcher.ru @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/examples/watcher_demo.ru b/examples/watcher_demo.ru index 91f4cca..7a6e675 100644 --- a/examples/watcher_demo.ru +++ b/examples/watcher_demo.ru @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/examples/yahns.conf.rb b/examples/yahns.conf.rb index f5b4f10..75f0bd1 100644 --- a/examples/yahns.conf.rb +++ b/examples/yahns.conf.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/examples/zbatery.conf.rb b/examples/zbatery.conf.rb index 5f94c0e..0537466 100644 --- a/examples/zbatery.conf.rb +++ b/examples/zbatery.conf.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/ext/raindrops/extconf.rb b/ext/raindrops/extconf.rb index b8f147c..b1310b0 100644 --- a/ext/raindrops/extconf.rb +++ b/ext/raindrops/extconf.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops.rb b/lib/raindrops.rb index dc61952..c071d57 100644 --- a/lib/raindrops.rb +++ b/lib/raindrops.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/aggregate.rb b/lib/raindrops/aggregate.rb index 4fb731f..9ed7eb7 100644 --- a/lib/raindrops/aggregate.rb +++ b/lib/raindrops/aggregate.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/aggregate/last_data_recv.rb b/lib/raindrops/aggregate/last_data_recv.rb index 32908f2..2205208 100644 --- a/lib/raindrops/aggregate/last_data_recv.rb +++ b/lib/raindrops/aggregate/last_data_recv.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/aggregate/pmq.rb b/lib/raindrops/aggregate/pmq.rb index 64d0a4f..94bdf4f 100644 --- a/lib/raindrops/aggregate/pmq.rb +++ b/lib/raindrops/aggregate/pmq.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/last_data_recv.rb b/lib/raindrops/last_data_recv.rb index b4808a1..e6c47e1 100644 --- a/lib/raindrops/last_data_recv.rb +++ b/lib/raindrops/last_data_recv.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/linux.rb b/lib/raindrops/linux.rb index 9842ae1..a76192c 100644 --- a/lib/raindrops/linux.rb +++ b/lib/raindrops/linux.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/middleware.rb b/lib/raindrops/middleware.rb index 20e573c..25b5a1e 100644 --- a/lib/raindrops/middleware.rb +++ b/lib/raindrops/middleware.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/middleware/proxy.rb b/lib/raindrops/middleware/proxy.rb index a7c8e66..433950c 100644 --- a/lib/raindrops/middleware/proxy.rb +++ b/lib/raindrops/middleware/proxy.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/struct.rb b/lib/raindrops/struct.rb index e81a78e..7233ce8 100644 --- a/lib/raindrops/struct.rb +++ b/lib/raindrops/struct.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/watcher.rb b/lib/raindrops/watcher.rb index ac5b895..8fc0772 100644 --- a/lib/raindrops/watcher.rb +++ b/lib/raindrops/watcher.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/setup.rb b/setup.rb index 5eb5006..b382468 100644 --- a/setup.rb +++ b/setup.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/ipv6_enabled.rb b/test/ipv6_enabled.rb index c4c9709..84ed9c1 100644 --- a/test/ipv6_enabled.rb +++ b/test/ipv6_enabled.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/rack_unicorn.rb b/test/rack_unicorn.rb index 0ecbd42..05a7751 100644 --- a/test/rack_unicorn.rb +++ b/test/rack_unicorn.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_aggregate_pmq.rb b/test/test_aggregate_pmq.rb index 692b9bd..24e0277 100644 --- a/test/test_aggregate_pmq.rb +++ b/test/test_aggregate_pmq.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_inet_diag_socket.rb b/test/test_inet_diag_socket.rb index a8c9973..e310dff 100644 --- a/test/test_inet_diag_socket.rb +++ b/test/test_inet_diag_socket.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_last_data_recv.rb b/test/test_last_data_recv.rb index 9643dc6..edd00f3 100644 --- a/test/test_last_data_recv.rb +++ b/test/test_last_data_recv.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_last_data_recv_unicorn.rb b/test/test_last_data_recv_unicorn.rb index 60d1be9..55f5e7f 100644 --- a/test/test_last_data_recv_unicorn.rb +++ b/test/test_last_data_recv_unicorn.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_linux.rb b/test/test_linux.rb index 7808469..5451c3f 100644 --- a/test/test_linux.rb +++ b/test/test_linux.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_linux_all_tcp_listen_stats.rb b/test/test_linux_all_tcp_listen_stats.rb index ef1f943..12a35ba 100644 --- a/test/test_linux_all_tcp_listen_stats.rb +++ b/test/test_linux_all_tcp_listen_stats.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_linux_all_tcp_listen_stats_leak.rb b/test/test_linux_all_tcp_listen_stats_leak.rb index 7be46d4..a3da07e 100644 --- a/test/test_linux_all_tcp_listen_stats_leak.rb +++ b/test/test_linux_all_tcp_listen_stats_leak.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_linux_ipv6.rb b/test/test_linux_ipv6.rb index 9e8730a..9ef8f0a 100644 --- a/test/test_linux_ipv6.rb +++ b/test/test_linux_ipv6.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_linux_middleware.rb b/test/test_linux_middleware.rb index f573225..7ed20df 100644 --- a/test/test_linux_middleware.rb +++ b/test/test_linux_middleware.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_linux_reuseport_tcp_listen_stats.rb b/test/test_linux_reuseport_tcp_listen_stats.rb index 4fda218..82083e0 100644 --- a/test/test_linux_reuseport_tcp_listen_stats.rb +++ b/test/test_linux_reuseport_tcp_listen_stats.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_middleware.rb b/test/test_middleware.rb index 56ce346..5694cd4 100644 --- a/test/test_middleware.rb +++ b/test/test_middleware.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_middleware_unicorn.rb b/test/test_middleware_unicorn.rb index 6730d4b..53226a9 100644 --- a/test/test_middleware_unicorn.rb +++ b/test/test_middleware_unicorn.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_middleware_unicorn_ipv6.rb b/test/test_middleware_unicorn_ipv6.rb index 3d6862c..99ecb7f 100644 --- a/test/test_middleware_unicorn_ipv6.rb +++ b/test/test_middleware_unicorn_ipv6.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_raindrops.rb b/test/test_raindrops.rb index 6351c66..165766e 100644 --- a/test/test_raindrops.rb +++ b/test/test_raindrops.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_raindrops_gc.rb b/test/test_raindrops_gc.rb index 2098129..a9f2026 100644 --- a/test/test_raindrops_gc.rb +++ b/test/test_raindrops_gc.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_struct.rb b/test/test_struct.rb index 9792d5b..abf0c59 100644 --- a/test/test_struct.rb +++ b/test/test_struct.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_tcp_info.rb b/test/test_tcp_info.rb index 2ddacfd..2dc5c50 100644 --- a/test/test_tcp_info.rb +++ b/test/test_tcp_info.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/test/test_watcher.rb b/test/test_watcher.rb index e5d1fa2..3cf667c 100644 --- a/test/test_watcher.rb +++ b/test/test_watcher.rb @@ -1,0 +2 @@ +# frozen_string_literal: false
Once again Ruby seems ready to introduce more incompatibilities and force busywork upon maintainers[1]. I definitely don't want to be dealing with this work in the future, so I used the following sh + ed invocation to prepend `frozen_string_literal: false' to every Ruby file to avoid breaking anything until somebody else has the opportunity to go through every single line of code. for i in "$@" do ed $i <<-EOM 0i # frozen_string_literal: false . w q EOM done [1] https://bugs.ruby-lang.org/issues/20205 --- examples/linux-listener-stats.rb | 1 + examples/yahns.conf.rb | 1 + examples/zbatery.conf.rb | 1 + ext/raindrops/extconf.rb | 1 + lib/raindrops.rb | 1 + lib/raindrops/aggregate.rb | 1 + lib/raindrops/aggregate/last_data_recv.rb | 1 + lib/raindrops/aggregate/pmq.rb | 1 + lib/raindrops/last_data_recv.rb | 1 + lib/raindrops/linux.rb | 1 + lib/raindrops/middleware.rb | 1 + lib/raindrops/middleware/proxy.rb | 1 + lib/raindrops/struct.rb | 1 + lib/raindrops/watcher.rb | 1 + setup.rb | 1 + test/ipv6_enabled.rb | 1 + test/rack_unicorn.rb | 1 + test/test_aggregate_pmq.rb | 1 + test/test_inet_diag_socket.rb | 1 + test/test_last_data_recv.rb | 1 + test/test_last_data_recv_unicorn.rb | 1 + test/test_linux.rb | 1 + test/test_linux_all_tcp_listen_stats.rb | 1 + test/test_linux_all_tcp_listen_stats_leak.rb | 1 + test/test_linux_ipv6.rb | 1 + test/test_linux_middleware.rb | 1 + test/test_linux_reuseport_tcp_listen_stats.rb | 1 + test/test_middleware.rb | 1 + test/test_middleware_unicorn.rb | 1 + test/test_middleware_unicorn_ipv6.rb | 1 + test/test_raindrops.rb | 1 + test/test_raindrops_gc.rb | 1 + test/test_struct.rb | 1 + test/test_tcp_info.rb | 1 + test/test_watcher.rb | 1 + 35 files changed, 35 insertions(+) diff --git a/examples/linux-listener-stats.rb b/examples/linux-listener-stats.rb index 7e767da..35908cf 100755 --- a/examples/linux-listener-stats.rb +++ b/examples/linux-listener-stats.rb @@ -1,0 +2 @@ +# frozen_string_literal: false diff --git a/examples/yahns.conf.rb b/examples/yahns.conf.rb index f5b4f10..75f0bd1 100644 --- a/examples/yahns.conf.rb +++ b/examples/yahns.conf.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/examples/zbatery.conf.rb b/examples/zbatery.conf.rb index 5f94c0e..0537466 100644 --- a/examples/zbatery.conf.rb +++ b/examples/zbatery.conf.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/ext/raindrops/extconf.rb b/ext/raindrops/extconf.rb index b8f147c..b1310b0 100644 --- a/ext/raindrops/extconf.rb +++ b/ext/raindrops/extconf.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops.rb b/lib/raindrops.rb index dc61952..6cdbdd5 100644 --- a/lib/raindrops.rb +++ b/lib/raindrops.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/aggregate.rb b/lib/raindrops/aggregate.rb index 4fb731f..3c274c4 100644 --- a/lib/raindrops/aggregate.rb +++ b/lib/raindrops/aggregate.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/aggregate/last_data_recv.rb b/lib/raindrops/aggregate/last_data_recv.rb index 32908f2..6e0d60f 100644 --- a/lib/raindrops/aggregate/last_data_recv.rb +++ b/lib/raindrops/aggregate/last_data_recv.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/aggregate/pmq.rb b/lib/raindrops/aggregate/pmq.rb index 64d0a4f..b97e64a 100644 --- a/lib/raindrops/aggregate/pmq.rb +++ b/lib/raindrops/aggregate/pmq.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/last_data_recv.rb b/lib/raindrops/last_data_recv.rb index b4808a1..247e5ac 100644 --- a/lib/raindrops/last_data_recv.rb +++ b/lib/raindrops/last_data_recv.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/linux.rb b/lib/raindrops/linux.rb index 9842ae1..27e6251 100644 --- a/lib/raindrops/linux.rb +++ b/lib/raindrops/linux.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/middleware.rb b/lib/raindrops/middleware.rb index 20e573c..d68fbf2 100644 --- a/lib/raindrops/middleware.rb +++ b/lib/raindrops/middleware.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/middleware/proxy.rb b/lib/raindrops/middleware/proxy.rb index a7c8e66..35e0e56 100644 --- a/lib/raindrops/middleware/proxy.rb +++ b/lib/raindrops/middleware/proxy.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/struct.rb b/lib/raindrops/struct.rb index e81a78e..68a4d9c 100644 --- a/lib/raindrops/struct.rb +++ b/lib/raindrops/struct.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/lib/raindrops/watcher.rb b/lib/raindrops/watcher.rb index ac5b895..ded434e 100644 --- a/lib/raindrops/watcher.rb +++ b/lib/raindrops/watcher.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/setup.rb b/setup.rb index 5eb5006..510a1a8 100644 --- a/setup.rb +++ b/setup.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/ipv6_enabled.rb b/test/ipv6_enabled.rb index c4c9709..84ed9c1 100644 --- a/test/ipv6_enabled.rb +++ b/test/ipv6_enabled.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/rack_unicorn.rb b/test/rack_unicorn.rb index 0ecbd42..9ef7fb5 100644 --- a/test/rack_unicorn.rb +++ b/test/rack_unicorn.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_aggregate_pmq.rb b/test/test_aggregate_pmq.rb index 692b9bd..24e0277 100644 --- a/test/test_aggregate_pmq.rb +++ b/test/test_aggregate_pmq.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_inet_diag_socket.rb b/test/test_inet_diag_socket.rb index a8c9973..a236d93 100644 --- a/test/test_inet_diag_socket.rb +++ b/test/test_inet_diag_socket.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_last_data_recv.rb b/test/test_last_data_recv.rb index 9643dc6..edd00f3 100644 --- a/test/test_last_data_recv.rb +++ b/test/test_last_data_recv.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_last_data_recv_unicorn.rb b/test/test_last_data_recv_unicorn.rb index 60d1be9..d99c250 100644 --- a/test/test_last_data_recv_unicorn.rb +++ b/test/test_last_data_recv_unicorn.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_linux.rb b/test/test_linux.rb index 7808469..2fa6f5a 100644 --- a/test/test_linux.rb +++ b/test/test_linux.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_linux_all_tcp_listen_stats.rb b/test/test_linux_all_tcp_listen_stats.rb index ef1f943..05d6aa9 100644 --- a/test/test_linux_all_tcp_listen_stats.rb +++ b/test/test_linux_all_tcp_listen_stats.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_linux_all_tcp_listen_stats_leak.rb b/test/test_linux_all_tcp_listen_stats_leak.rb index 7be46d4..f7e2020 100644 --- a/test/test_linux_all_tcp_listen_stats_leak.rb +++ b/test/test_linux_all_tcp_listen_stats_leak.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_linux_ipv6.rb b/test/test_linux_ipv6.rb index 9e8730a..041cf33 100644 --- a/test/test_linux_ipv6.rb +++ b/test/test_linux_ipv6.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_linux_middleware.rb b/test/test_linux_middleware.rb index f573225..1b3cea3 100644 --- a/test/test_linux_middleware.rb +++ b/test/test_linux_middleware.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_linux_reuseport_tcp_listen_stats.rb b/test/test_linux_reuseport_tcp_listen_stats.rb index 4fda218..926bbb7 100644 --- a/test/test_linux_reuseport_tcp_listen_stats.rb +++ b/test/test_linux_reuseport_tcp_listen_stats.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_middleware.rb b/test/test_middleware.rb index 56ce346..e85c0f0 100644 --- a/test/test_middleware.rb +++ b/test/test_middleware.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_middleware_unicorn.rb b/test/test_middleware_unicorn.rb index 6730d4b..9ae94ad 100644 --- a/test/test_middleware_unicorn.rb +++ b/test/test_middleware_unicorn.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_middleware_unicorn_ipv6.rb b/test/test_middleware_unicorn_ipv6.rb index 3d6862c..11d9af8 100644 --- a/test/test_middleware_unicorn_ipv6.rb +++ b/test/test_middleware_unicorn_ipv6.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_raindrops.rb b/test/test_raindrops.rb index 6351c66..a82ab99 100644 --- a/test/test_raindrops.rb +++ b/test/test_raindrops.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_raindrops_gc.rb b/test/test_raindrops_gc.rb index 2098129..323185b 100644 --- a/test/test_raindrops_gc.rb +++ b/test/test_raindrops_gc.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_struct.rb b/test/test_struct.rb index 9792d5b..abf0c59 100644 --- a/test/test_struct.rb +++ b/test/test_struct.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_tcp_info.rb b/test/test_tcp_info.rb index 2ddacfd..8d24b94 100644 --- a/test/test_tcp_info.rb +++ b/test/test_tcp_info.rb @@ -0,0 +1 @@ +# frozen_string_literal: false diff --git a/test/test_watcher.rb b/test/test_watcher.rb index e5d1fa2..0fafb6d 100644 --- a/test/test_watcher.rb +++ b/test/test_watcher.rb @@ -0,0 +1 @@ +# frozen_string_literal: false
Hopefully this doesn't cause more breakage and we'll never actually need a rack 4 --- raindrops.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/raindrops.gemspec b/raindrops.gemspec index 1de56a0..2f171db 100644 --- a/raindrops.gemspec +++ b/raindrops.gemspec @@ -21,6 +21,6 @@ s.add_development_dependency('aggregate', '~> 0.2') s.add_development_dependency('test-unit', '~> 3.0') s.add_development_dependency('posix_mq', '~> 2.0') - s.add_development_dependency('rack', [ '>= 1.2', '< 3.0' ]) + s.add_development_dependency('rack', [ '>= 1.2', '< 4' ]) s.licenses = %w(LGPL-2.1+) end
Compiler optimization isn't useful when doing portability checks for any of the things we care about. --- ext/raindrops/extconf.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/raindrops/extconf.rb b/ext/raindrops/extconf.rb index 1733703..b8f147c 100644 --- a/ext/raindrops/extconf.rb +++ b/ext/raindrops/extconf.rb @@ -1,6 +1,7 @@ require 'mkmf' require 'shellwords' +$CFLAGS += ' -O0 ' # faster checks dir_config('atomic_ops') have_func('mmap', 'sys/mman.h') or abort 'mmap() not found' have_func('munmap', 'sys/mman.h') or abort 'munmap() not found' @@ -158,4 +159,5 @@ apt-get install libatomic-ops-dev SRC create_header # generate extconf.h to avoid excessively long command-line +$CFLAGS.sub!(/ -O0 /, '') create_makefile('raindrops_ext')
posix_mq is rarely installed, so don't force users to have it in order to test or develop raindrops. --- test/test_last_data_recv.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/test_last_data_recv.rb b/test/test_last_data_recv.rb index b1a5ac6..9643dc6 100644 --- a/test/test_last_data_recv.rb +++ b/test/test_last_data_recv.rb @@ -9,6 +9,14 @@ require 'io/wait' class TestLastDataRecv < Test::Unit::TestCase + def setup + Raindrops::Aggregate::LastDataRecv.default_aggregate = [] + end + + def teardown + Raindrops::Aggregate::LastDataRecv.default_aggregate = nil + end + def test_accept_nonblock_agg s = Socket.new(:INET, :STREAM, 0) s.listen(128)
From: Eric Wong <e@80x24.org> Diskspace and bandwidth are expensive, and we can make rack+aggregate optional in tests, too. --- test/rack_unicorn.rb | 3 +-- test/test_last_data_recv.rb | 8 +++++++- test/test_watcher.rb | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/test/rack_unicorn.rb b/test/rack_unicorn.rb index 779e8bf..0ecbd42 100644 --- a/test/rack_unicorn.rb +++ b/test/rack_unicorn.rb @@ -1,11 +1,10 @@ # -*- encoding: binary -*- require "test/unit" require "raindrops" -require "rack" -require "rack/lobster" require "open-uri" begin require "unicorn" + require "rack" require "rack/lobster" rescue LoadError => e warn "W: #{e} skipping test since Rack or Unicorn was not found" diff --git a/test/test_last_data_recv.rb b/test/test_last_data_recv.rb index ef84e05..b1a5ac6 100644 --- a/test/test_last_data_recv.rb +++ b/test/test_last_data_recv.rb @@ -1,3 +1,9 @@ +begin + require 'aggregate' + have_aggregate = true +rescue LoadError => e + warn "W: #{e} skipping #{__FILE__}" +end require 'test/unit' require 'raindrops' require 'io/wait' @@ -40,4 +46,4 @@ def test_accept_nonblock_one assert_equal 1, s.raindrops_aggregate.size assert_raise(IO::WaitReadable) { s.accept_nonblock } end -end if RUBY_PLATFORM =~ /linux/ +end if RUBY_PLATFORM =~ /linux/ && have_aggregate diff --git a/test/test_watcher.rb b/test/test_watcher.rb index 28ac49b..e5d1fa2 100644 --- a/test/test_watcher.rb +++ b/test/test_watcher.rb @@ -1,9 +1,9 @@ # -*- encoding: binary -*- require "test/unit" -require "rack" require "raindrops" begin require 'aggregate' + require 'rack' rescue LoadError => e warn "W: #{e} skipping #{__FILE__}" end @@ -183,4 +183,4 @@ def test_peaks assert_equal queued_before, headers["X-Last-Peak-At"], "should not change" assert_equal start, headers["X-First-Peak-At"] end -end if RUBY_PLATFORM =~ /linux/ && defined?(Aggregate) +end if RUBY_PLATFORM =~ /linux/ && defined?(Aggregate) && defined?(Rack)
Some things I noticed while preparing a release in the wake of Ruby 3.3. Eric Wong (4): tests: support running tests without rack||aggregate test/test_last_data_recv: don't require posix_mq extconf: disable optimization to speed up checks by ~3% gemspec: support rack 3.x ext/raindrops/extconf.rb | 2 ++ raindrops.gemspec | 2 +- test/rack_unicorn.rb | 3 +-- test/test_last_data_recv.rb | 16 +++++++++++++++- test/test_watcher.rb | 4 ++-- 5 files changed, 21 insertions(+), 6 deletions(-)
Eric Wong <bofh@yhbt.net> wrote: > I'll squash this in for fork+preload safety. I forget there are multithreaded servers using this :x ------8<----- Subject: [PATCH v3] middleware: reuse inet_diag netlink socket No point in constantly allocating and deallocating FDs (and Ruby IO objects) when reusing them is supported. However, we must guard it against parallel callers since linux_inet_diag.c::diag releases the GVL while calling sendmsg(2) and recvmsg(2), so it's possible two Ruby threads can end up crossing streams if the middlware is used with a multi-threaded server. AFAIK, Raindrops::Middleware isn't a heavily-trafficked endpoint, so saving FDs and avoiding thread specific data is preferable for memory-constrained systems I use. --- lib/raindrops/middleware.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/raindrops/middleware.rb b/lib/raindrops/middleware.rb index d5e3927..20e573c 100644 --- a/lib/raindrops/middleware.rb +++ b/lib/raindrops/middleware.rb @@ -1,5 +1,6 @@ # -*- encoding: binary -*- require 'raindrops' +require 'thread' # Raindrops::Middleware is Rack middleware that allows snapshotting # current activity from an HTTP request. For all operating systems, @@ -93,11 +94,12 @@ def initialize(app, opts = {}) @app = app @stats = opts[:stats] || Stats.new @path = opts[:path] || "/_raindrops" + @mtx = Mutex.new tmp = opts[:listeners] if tmp.nil? && defined?(Unicorn) && Unicorn.respond_to?(:listener_names) tmp = Unicorn.listener_names end - @tcp = @unix = nil + @nl_sock = @tcp = @unix = nil if tmp @tcp = tmp.grep(/\A.+:\d+\z/) @@ -129,9 +131,12 @@ def stats_response # :nodoc: "writing: #{@stats.writing}\n" if defined?(Raindrops::Linux.tcp_listener_stats) - Raindrops::Linux.tcp_listener_stats(@tcp).each do |addr,stats| - body << "#{addr} active: #{stats.active}\n" \ - "#{addr} queued: #{stats.queued}\n" + @mtx.synchronize do + @nl_sock ||= Raindrops::InetDiagSocket.new + Raindrops::Linux.tcp_listener_stats(@tcp, @nl_sock).each do |addr,stats| + body << "#{addr} active: #{stats.active}\n" \ + "#{addr} queued: #{stats.queued}\n" + end end if @tcp Raindrops::Linux.unix_listener_stats(@unix).each do |addr,stats| body << "#{addr} active: #{stats.active}\n" \
I'll squash this in for fork+preload safety. Fwiw, most of the stats stuff has never seen any real-world use to my knowledge :x Somebody else got me to work on it but never got around to using it, so you're probably the only guinea pig :> diff --git a/lib/raindrops/middleware.rb b/lib/raindrops/middleware.rb index e0781f2..a9e2ee3 100644 --- a/lib/raindrops/middleware.rb +++ b/lib/raindrops/middleware.rb @@ -98,8 +98,6 @@ def initialize(app, opts = {}) tmp = Unicorn.listener_names end @nl_sock = @tcp = @unix = nil - defined?(Raindrops::Linux.tcp_listener_stats) and - @nl_sock = Raindrops::InetDiagSocket.new if tmp @tcp = tmp.grep(/\A.+:\d+\z/) @@ -131,6 +129,7 @@ def stats_response # :nodoc: "writing: #{@stats.writing}\n" if defined?(Raindrops::Linux.tcp_listener_stats) + @nl_sock ||= Raindrops::InetDiagSocket.new Raindrops::Linux.tcp_listener_stats(@tcp, @nl_sock).each do |addr,stats| body << "#{addr} active: #{stats.active}\n" \ "#{addr} queued: #{stats.queued}\n"
We should warn gracefully when we hit IPv7+ or whatever... --- ext/raindrops/linux_inet_diag.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ext/raindrops/linux_inet_diag.c b/ext/raindrops/linux_inet_diag.c index e4050cb..6d421e7 100644 --- a/ext/raindrops/linux_inet_diag.c +++ b/ext/raindrops/linux_inet_diag.c @@ -235,7 +235,9 @@ static struct listen_stats *stats_for(st_table *table, struct inet_diag_msg *r) break; } default: - assert(0 && "unsupported address family, could that be IPv7?!"); + fprintf(stderr, "unsupported .idiag_family: %u\n", + (unsigned)r->idiag_family); + return NULL; /* can't raise w/o GVL */ } if (!inet_ntop(r->idiag_family, src, host, hostlen)) { bug_warn_nogvl("BUG: inet_ntop: %s\n", strerror(errno)); @@ -255,7 +257,8 @@ static struct listen_stats *stats_for(st_table *table, struct inet_diag_msg *r) port = host + hostlen + 2; break; default: - assert(0 && "unsupported address family, could that be IPv7?!"); + assert(0 && "should never get here (returned above)"); + abort(); } n = snprintf(port, portlen, "%u", ntohs(r->id.idiag_sport)); @@ -300,12 +303,14 @@ static struct listen_stats *stats_for(st_table *table, struct inet_diag_msg *r) static void table_incr_active(st_table *table, struct inet_diag_msg *r) { struct listen_stats *stats = stats_for(table, r); + if (!stats) return; ++stats->active; } static void table_set_queued(st_table *table, struct inet_diag_msg *r) { struct listen_stats *stats = stats_for(table, r); + if (!stats) return; stats->listener_p = 1; stats->queued += r->idiag_rqueue; }
> Your code (and some of the Ruby code shipped w/ raindrops) > should probably be reusing sockets given our API allows it. IOW, something like this (don't think test cases are worth updating): ------8<------- Subject: [PATCH] middleware: reuse inet_diag netlink socket No point in constantly allocating and deallocating FDs (and Ruby IO objects) when reusing them is supported. --- lib/raindrops/middleware.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/raindrops/middleware.rb b/lib/raindrops/middleware.rb index d5e3927..e0781f2 100644 --- a/lib/raindrops/middleware.rb +++ b/lib/raindrops/middleware.rb @@ -97,7 +97,9 @@ def initialize(app, opts = {}) if tmp.nil? && defined?(Unicorn) && Unicorn.respond_to?(:listener_names) tmp = Unicorn.listener_names end - @tcp = @unix = nil + @nl_sock = @tcp = @unix = nil + defined?(Raindrops::Linux.tcp_listener_stats) and + @nl_sock = Raindrops::InetDiagSocket.new if tmp @tcp = tmp.grep(/\A.+:\d+\z/) @@ -129,7 +131,7 @@ def stats_response # :nodoc: "writing: #{@stats.writing}\n" if defined?(Raindrops::Linux.tcp_listener_stats) - Raindrops::Linux.tcp_listener_stats(@tcp).each do |addr,stats| + Raindrops::Linux.tcp_listener_stats(@tcp, @nl_sock).each do |addr,stats| body << "#{addr} active: #{stats.active}\n" \ "#{addr} queued: #{stats.queued}\n" end if @tcp
Jean Boussier <jean.boussier@gmail.com> wrote: > Hello, > > Once again apologies for not submitting the patch in an usable format. <snip> I was actually able to run `git am' on your raw mail just fine w/o needing an active connection. So whatever you did worked[1] More inline... <snip> > Looking at the raindrops implementation it seems to assume > the GC will take care of regularly closing these sockets, but > I think it’s a bit too bold of an assumption. Your code (and some of the Ruby code shipped w/ raindrops) should probably be reusing sockets given our API allows it. At the top of the tcp_listener_stats function, we have: rb_scan_args(argc, argv, "02", &addrs, &sock); and then: sock = NIL_P(sock) ? rb_funcall(cIDSock, id_new, 0) : rb_io_get_io(sock); So we only create sockets one isn't passed. > diff --git a/ext/raindrops/linux_inet_diag.c b/ext/raindrops/linux_inet_diag.c > index 2a2360c..b3d9a51 100644 > --- a/ext/raindrops/linux_inet_diag.c > +++ b/ext/raindrops/linux_inet_diag.c > @@ -634,7 +634,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) > switch (TYPE(addrs)) { > case T_STRING: > rb_hash_aset(rv, addrs, tcp_stats(&args, addrs)); > - return rv; > + goto out; OK > @@ -643,7 +643,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) > VALUE cur = rb_ary_entry(addrs, 0); > > rb_hash_aset(rv, cur, tcp_stats(&args, cur)); > - return rv; > + goto out; OK > } > for (i = 0; i < len; i++) { > union any_addr check; > @@ -659,6 +659,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) > gen_bytecode_all(&args.iov[2]); > break; > default: > + rb_io_close(sock); That needs the (argc < 2) guard like below in `out:'. We should never close sockets passed by the user. Indentation is also done with hard tabs for this project (and alignment with spaces, roughly git.git and Linux kernel style)[2]. I'll push out the patch below if it looks OK to you. > rb_raise(rb_eArgError, > "addr must be an array of strings, a string, or nil"); > } > @@ -671,6 +672,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) > if (RHASH_SIZE(rv) > 1) > rb_hash_foreach(rv, drop_placeholders, Qfalse); > > +out: OK. > /* let GC deal with corner cases */ > if (argc < 2) rb_io_close(sock); > return rv; That 'if (argc < 2)' is important for the exception above. This is what I'll push out: -----8<----- From: Jean Boussier <jean.boussier@gmail.com> Subject: [PATCH] tcp_listener_stats: always eagerly close sockets I just debugged an issue with our system, I was witnessing the number of file descriptor in our process grow at an alarming rate which I mapped to our use of raindrops to report utilisation. For various reasons we don’t call raindrops from a Rack middleware but have one process that monitor the socket continuously, and share that data with the workers. Since we call tcp_listener_stats every seconds in a process that doesn't do much else, GC very rarely triggers if at all which cause `InetDiagSocket` instances to accumulate very quickly. Each of those instances holds a file descriptor. Looking at the raindrops implementation it seems to assume the GC will take care of regularly closing these sockets, but I think it’s a bit too bold of an assumption. [ew: don't close user-passed sockets on exception] Acked-by: Eric Wong <e@80x24.org> --- Range-diff: 1: 6a93833 ! 1: 9b9909b tcp_listener_stats: always eagerly close sockets @@ Commit message the GC will take care of regularly closing these sockets, but I think it’s a bit too bold of an assumption. + [ew: don't close user-passed sockets on exception] + + Acked-by: Eric Wong <e@80x24.org> + ## ext/raindrops/linux_inet_diag.c ## @@ ext/raindrops/linux_inet_diag.c: static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) switch (TYPE(addrs)) { @@ ext/raindrops/linux_inet_diag.c: static VALUE tcp_listener_stats(int argc, VALUE gen_bytecode_all(&args.iov[2]); break; default: -+ rb_io_close(sock); ++ if (argc < 2) rb_io_close(sock); rb_raise(rb_eArgError, "addr must be an array of strings, a string, or nil"); } ext/raindrops/linux_inet_diag.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ext/raindrops/linux_inet_diag.c b/ext/raindrops/linux_inet_diag.c index 2d4f503..e4050cb 100644 --- a/ext/raindrops/linux_inet_diag.c +++ b/ext/raindrops/linux_inet_diag.c @@ -636,7 +636,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) switch (TYPE(addrs)) { case T_STRING: rb_hash_aset(rv, addrs, tcp_stats(&args, addrs)); - return rv; + goto out; case T_ARRAY: { long i; long len = RARRAY_LEN(addrs); @@ -645,7 +645,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) VALUE cur = rb_ary_entry(addrs, 0); rb_hash_aset(rv, cur, tcp_stats(&args, cur)); - return rv; + goto out; } for (i = 0; i < len; i++) { union any_addr check; @@ -661,6 +661,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) gen_bytecode_all(&args.iov[2]); break; default: + if (argc < 2) rb_io_close(sock); rb_raise(rb_eArgError, "addr must be an array of strings, a string, or nil"); } @@ -673,6 +674,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) if (RHASH_SIZE(rv) > 1) rb_hash_foreach(rv, drop_placeholders, Qfalse); +out: /* let GC deal with corner cases */ rb_str_resize(buf, 0); if (argc < 2) rb_io_close(sock); [1] Fwiw, the mail submission port is open on yhbt.net and you can use `git send-email' with it: git send-email \ --smtp-domain=yhbt.net \ --smtp-debug=1 \ --smtp-encryption=tls \ --smtp-server-port=587 \ --smtp-server=yhbt.net \ --to raindrops-public@yhbt.net \ --suppress-cc=all /path/to/patches If you prefer pull requests, format messages with the "git request-pull" command so they're easy to search for[3]. Sorry, but using a proprietary+centralized hosting service owned by a convicted monopolist puts me in a bad mood, especially when I'm to blame for their success given my involvement in git.git. repo.or.cz and Sourcehut are 100% Free Software if you don't feel like self-hosting. [2] yes, tabs were roughly ~16% faster for `git grep' https://lore.kerne.org/git/20071018024553.GA5186@coredump.intra.peff.net/ [3] There are automated bots and search queries that can search for these in mail archives. While none are currently in use for this project, https://yhbt.net/raindrops.git will probably feature it in the nearish future.
Hello, Once again apologies for not submitting the patch in an usable format. It can be downloaded with curl/wget from https://github.com/casperisfine/raindrops/commit/1c92b440ad7b11a1708a1d5ed75b0767f213b40a.patch I just debugged an issue with our system, I was witnessing the number of file descriptor in our process grow at an alarming rate which I mapped to our use of raindrops to report utilisation. For various reasons we don’t call raindrops from a Rack middleware but have one process that monitor the socket continuously, and share that data with the workers. Since we call tcp_listener_stats every seconds in a process that doesn't do much else, GC very rarely triggers if at all which cause `InetDiagSocket` instances to accumulate very quickly. Each of those instances holds a file descriptor. Looking at the raindrops implementation it seems to assume the GC will take care of regularly closing these sockets, but I think it’s a bit too bold of an assumption. Regards. --- ext/raindrops/linux_inet_diag.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ext/raindrops/linux_inet_diag.c b/ext/raindrops/linux_inet_diag.c index 2a2360c..b3d9a51 100644 --- a/ext/raindrops/linux_inet_diag.c +++ b/ext/raindrops/linux_inet_diag.c @@ -634,7 +634,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) switch (TYPE(addrs)) { case T_STRING: rb_hash_aset(rv, addrs, tcp_stats(&args, addrs)); - return rv; + goto out; case T_ARRAY: { long i; long len = RARRAY_LEN(addrs); @@ -643,7 +643,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) VALUE cur = rb_ary_entry(addrs, 0); rb_hash_aset(rv, cur, tcp_stats(&args, cur)); - return rv; + goto out; } for (i = 0; i < len; i++) { union any_addr check; @@ -659,6 +659,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) gen_bytecode_all(&args.iov[2]); break; default: + rb_io_close(sock); rb_raise(rb_eArgError, "addr must be an array of strings, a string, or nil"); } @@ -671,6 +672,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) if (RHASH_SIZE(rv) > 1) rb_hash_foreach(rv, drop_placeholders, Qfalse); +out: /* let GC deal with corner cases */ if (argc < 2) rb_io_close(sock); return rv;
Socket#accept and Socket#accept_nonblock return an Addrinfo object in addition to a client socket. This allows web servers to avoid having to make getpeername(2) syscalls to get the same information. --- lib/raindrops/aggregate/last_data_recv.rb | 23 ++++++++---- test/test_last_data_recv.rb | 43 +++++++++++++++++++++++ 2 files changed, 60 insertions(+), 6 deletions(-) create mode 100644 test/test_last_data_recv.rb diff --git a/lib/raindrops/aggregate/last_data_recv.rb b/lib/raindrops/aggregate/last_data_recv.rb index 6919fbc..32908f2 100644 --- a/lib/raindrops/aggregate/last_data_recv.rb +++ b/lib/raindrops/aggregate/last_data_recv.rb @@ -10,6 +10,8 @@ # Methods wrapped include: # - TCPServer#accept # - TCPServer#accept_nonblock +# - Socket#accept +# - Socket#accept_nonblock # - Kgio::TCPServer#kgio_accept # - Kgio::TCPServer#kgio_tryaccept module Raindrops::Aggregate::LastDataRecv @@ -33,8 +35,10 @@ def self.default_aggregate=(agg) # automatically extends any TCPServer objects used by Unicorn def self.cornify! - Unicorn::HttpServer::LISTENERS.each do |sock| - sock.extend(self) if TCPServer === sock + Unicorn::HttpServer::LISTENERS.each do |s| + if TCPServer === s || (s.instance_of?(Socket) && s.local_address.ip?) + s.extend(self) + end end end @@ -60,8 +64,8 @@ def accept count! super end - def accept_nonblock - count! super + def accept_nonblock(exception: true) + count! super(exception: exception) end # :startdoc: @@ -72,12 +76,19 @@ def accept_nonblock # # We require TCP_DEFER_ACCEPT on the listen socket for # +last_data_recv+ to be accurate - def count!(io) + def count!(ret) + case ret + when :wait_readable + when Array # Socket#accept_nonblock + io = ret[0] + else # TCPSocket#accept_nonblock + io = ret + end if io x = Raindrops::TCP_Info.new(io) @raindrops_aggregate << x.last_data_recv end - io + ret end end diff --git a/test/test_last_data_recv.rb b/test/test_last_data_recv.rb new file mode 100644 index 0000000..ef84e05 --- /dev/null +++ b/test/test_last_data_recv.rb @@ -0,0 +1,43 @@ +require 'test/unit' +require 'raindrops' +require 'io/wait' + +class TestLastDataRecv < Test::Unit::TestCase + def test_accept_nonblock_agg + s = Socket.new(:INET, :STREAM, 0) + s.listen(128) + addr = s.connect_address + s.extend(Raindrops::Aggregate::LastDataRecv) + s.raindrops_aggregate = [] + c = Socket.new(:INET, :STREAM, 0) + c.connect(addr) + c.write '.' # for TCP_DEFER_ACCEPT + client, ai = s.accept_nonblock(exception: false) + assert client.kind_of?(Socket) + assert ai.kind_of?(Addrinfo) + assert_equal 1, s.raindrops_aggregate.size + assert s.raindrops_aggregate[0].instance_of?(Integer) + client, ai = s.accept_nonblock(exception: false) + assert_equal :wait_readable, client + assert_nil ai + assert_equal 1, s.raindrops_aggregate.size + assert_raise(IO::WaitReadable) { s.accept_nonblock } + end + + def test_accept_nonblock_one + s = TCPServer.new('127.0.0.1', 0) + s.extend(Raindrops::Aggregate::LastDataRecv) + s.raindrops_aggregate = [] + addr = s.addr + c = TCPSocket.new(addr[3], addr[1]) + c.write '.' # for TCP_DEFER_ACCEPT + client = s.accept_nonblock(exception: false) + assert client.kind_of?(TCPSocket) + assert_equal 1, s.raindrops_aggregate.size + assert s.raindrops_aggregate[0].instance_of?(Integer) + client = s.accept_nonblock(exception: false) + assert_equal :wait_readable, client + assert_equal 1, s.raindrops_aggregate.size + assert_raise(IO::WaitReadable) { s.accept_nonblock } + end +end if RUBY_PLATFORM =~ /linux/
> If you can guarantee the Ruby doesn't introduce further
> incompatibilities (the longer the guarantee, the better).
The plan is as follows:
- Ruby 3.3: deprecations on `struct rb_io`.
- Ruby 3.4+: remove public visibility of `struct rb_io`.
The reason for this change is to make it possible for us to change the internal details of `struct rb_io`. It’s also easier for other implementations e.g. JRuby, TruffleRuby, which don’t necessarily have the same layout and need to emulate it for native extensions.
We just merged a PR in Ruby head which amounts to:
#define HAVE_RB_IO_T
This restores the previous code path and makes Ruby 3.3 more compatible with existing code.
That being said, I think it would be great to merge the appropriate fixes which remove usage of deprecated fields in Ruby 3.3.
So to summarise, Ruby 3.3 should be compatible, but with deprecations. We don’t plan on introducing any further breaking changes until a later release (e.g 3.4 or later). If you can merge and release the appropriate patches, that would be greatly appreciated.
Warm regards,
Samuel
Samuel Williams <samuel.williams@oriontransfer.co.nz> wrote: > Hi Eric, > > Thanks for your continued effort to maintain and support this gem. I don't hate this gem as much as the others, but it's still uncomfortable for me to hear praise. > We are trying to clean up the `io.h` interface in Ruby 3.3 and > would like to know if there is anything we can do to help with > a release of Raindrops which supports the updated interface > and removes usage of deprecated symbols. As I wrote in kgio-public[1] about other Ruby 3.3+ changes. I also wonder if st.h hash table usage is on the chopping block at some point; and I'm also somewhat tempted to replace it anyways since I'll probably want to use some of this outside of Ruby. > I believe we’ve supplied a patch, but it is not formally > released yet. Is there anything we can do to help? If you can guarantee the Ruby doesn't introduce further incompatibilities (the longer the guarantee, the better). [1] https://yhbt.net/kgio-public/20230828005810.M622269@dcvr/
Hi Eric, Thanks for your continued effort to maintain and support this gem. We are trying to clean up the `io.h` interface in Ruby 3.3 and would like to know if there is anything we can do to help with a release of Raindrops which supports the updated interface and removes usage of deprecated symbols. I believe we’ve supplied a patch, but it is not formally released yet. Is there anything we can do to help? Warm regards, Samuel
Jean Boussier <jean.boussier@gmail.com> wrote: > > the "why" > > I think the missing piece here is that this change is necessary for compatibility with the current ruby master: > > https://bugs.ruby-lang.org/issues/19057#note-17 Thanks, that's an important note omitted from both original patches. I've pushed out Samuel's patch as commit b3212417cc3e7cc44aa9e1ffe89b0d62ef3fdab5 to raindrops.git with your comment > kgio seem to have the same code and fail in the same way. +Cc kgio-public@yhbt.net I'll take documented patches for kgio, otherwise I'll try to deal with it before Ruby 3.3 (or when Debian stable ships Ruby 3.3 :P). I've also posted some raindrops cleanups and modernizations which should reduce stack frames and binary size for 3.1+ (untested since I don't have space/time for multiple Ruby installs atm): https://yhbt.net/raindrops-public/20230611213328.379546-1-bofh@yhbt.net/
We don't list `unicorn' as a development dependency since unicorn currently depends on this project. While unicorn might drop us as a dependency, don't waste disk space and bandwidth of potential raindrops hackers who don't have unicorn. --- test/test_linux_reuseport_tcp_listen_stats.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_linux_reuseport_tcp_listen_stats.rb b/test/test_linux_reuseport_tcp_listen_stats.rb index 73995ea..4fda218 100644 --- a/test/test_linux_reuseport_tcp_listen_stats.rb +++ b/test/test_linux_reuseport_tcp_listen_stats.rb @@ -48,4 +48,4 @@ def test_reuseport_queue_stats assert_equal [0, i+1], all[addr].to_a end end -end if RUBY_PLATFORM =~ /linux/ +end if RUBY_PLATFORM =~ /linux/ && Object.const_defined?(:Unicorn)
alloca makes stack usage unpredictable and life difficult for static analysis tools and compilers. The 46 bytes of INET6_ADDRSTRLEN is fine to keep on stack, but page size can be several MB large in some architectures (but typically 4K on common architectures). Thus we handle page size-ed allocations via `rb_str_tmp_new'. `rb_str_tmp_new' has been in public Ruby headers since the 1.9 days and used by the core `zlib', `digest', and `zlib' extensions, so it should be safe to use (and `rb_str_resize' is used in many more C extensions). --- ext/raindrops/linux_inet_diag.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/ext/raindrops/linux_inet_diag.c b/ext/raindrops/linux_inet_diag.c index e1ae62a..2d4f503 100644 --- a/ext/raindrops/linux_inet_diag.c +++ b/ext/raindrops/linux_inet_diag.c @@ -212,24 +212,25 @@ static void bug_warn_nogvl(const char *fmt, ...) static struct listen_stats *stats_for(st_table *table, struct inet_diag_msg *r) { char *host, *key, *port, *old_key; - size_t alloca_len; struct listen_stats *stats; socklen_t hostlen; socklen_t portlen = (socklen_t)sizeof("65535"); int n; const void *src = r->id.idiag_src; + char buf[INET6_ADDRSTRLEN]; + size_t buf_len; switch (r->idiag_family) { case AF_INET: { hostlen = INET_ADDRSTRLEN; - alloca_len = hostlen + portlen; - host = key = alloca(alloca_len); + buf_len = hostlen + portlen; + host = key = buf; break; } case AF_INET6: { hostlen = INET6_ADDRSTRLEN; - alloca_len = 1 + hostlen + 1 + portlen; - key = alloca(alloca_len); + buf_len = 1 + hostlen + 1 + portlen; + key = buf; host = key + 1; break; } @@ -269,7 +270,7 @@ static struct listen_stats *stats_for(st_table *table, struct inet_diag_msg *r) old_key = key; if (r->idiag_state == TCP_ESTABLISHED) { - n = snprintf(key, alloca_len, "%s:%u", + n = snprintf(key, buf_len, "%s:%u", addr_any(r->idiag_family), ntohs(r->id.idiag_sport)); if (n <= 0) { @@ -615,7 +616,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) { VALUE rv = rb_hash_new(); struct nogvl_args args; - VALUE addrs, sock; + VALUE addrs, sock, buf; rb_scan_args(argc, argv, "02", &addrs, &sock); @@ -624,8 +625,9 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) * buffer for recvmsg() later, we already checked for * OPLEN <= page_size at initialization */ + buf = rb_str_buf_new(page_size); args.iov[2].iov_len = OPLEN; - args.iov[2].iov_base = alloca(page_size); + args.iov[2].iov_base = RSTRING_PTR(buf); args.table = NULL; sock = NIL_P(sock) ? rb_funcall(cIDSock, id_new, 0) : rb_io_get_io(sock); @@ -672,6 +674,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) rb_hash_foreach(rv, drop_placeholders, Qfalse); /* let GC deal with corner cases */ + rb_str_resize(buf, 0); if (argc < 2) rb_io_close(sock); return rv; }
This is less code and hopefully smaller binaries. `rb_io_check_closed' has been in Ruby since the pre-CVS of decades ago, and it doesn't matter if it's removed or not in the future since Ruby 3.1+ doesn't see this code path and calls `rb_io_descriptor' directly. --- ext/raindrops/my_fileno.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/raindrops/my_fileno.h b/ext/raindrops/my_fileno.h index 646f31b..3a0100f 100644 --- a/ext/raindrops/my_fileno.h +++ b/ext/raindrops/my_fileno.h @@ -9,9 +9,8 @@ static int my_fileno(VALUE io) rb_io_t *fptr; GetOpenFile(io, fptr); + rb_io_check_closed(fptr); - if (fptr->fd < 0) - rb_raise(rb_eIOError, "closed stream"); return fptr->fd; } #endif /* Ruby <3.1 !HAVE_RB_IO_DESCRIPTOR */
Calling `#to_io' is only necessary when we're handling an argument from user code where the user could pass a non-IO object. `#to_io' calls are a waste of time when we create the IO object ourselves (in `Raindrops::InetDiagSock.new'). This allows us to define the `my_fileno' macro for Ruby 3.1+ users to call the new `rb_io_descriptor' function directly without an extra C stack frame. This also allows us to get rid of nesting CPP directives inside C functions which (IMHO) improves readability. Furthermore, any necessary #to_io calls using `rb_convert_type' can be replaced with `rb_io_get_io' to decrease code size. `rb_io_get_io' has been in ruby/io.h since Ruby 1.9.2 and there's no expectation that it'd be deprecated since it only deals with opaque `VALUE' types. --- ext/raindrops/linux_inet_diag.c | 4 ++-- ext/raindrops/my_fileno.h | 13 ++++--------- ext/raindrops/tcp_info.c | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/ext/raindrops/linux_inet_diag.c b/ext/raindrops/linux_inet_diag.c index 2a2360c..e1ae62a 100644 --- a/ext/raindrops/linux_inet_diag.c +++ b/ext/raindrops/linux_inet_diag.c @@ -627,8 +627,8 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) args.iov[2].iov_len = OPLEN; args.iov[2].iov_base = alloca(page_size); args.table = NULL; - if (NIL_P(sock)) - sock = rb_funcall(cIDSock, id_new, 0); + sock = NIL_P(sock) ? rb_funcall(cIDSock, id_new, 0) + : rb_io_get_io(sock); args.fd = my_fileno(sock); switch (TYPE(addrs)) { diff --git a/ext/raindrops/my_fileno.h b/ext/raindrops/my_fileno.h index 00c5d29..646f31b 100644 --- a/ext/raindrops/my_fileno.h +++ b/ext/raindrops/my_fileno.h @@ -1,22 +1,17 @@ #include <ruby.h> #include <ruby/io.h> +#ifdef HAVE_RB_IO_DESCRIPTOR +# define my_fileno(io) rb_io_descriptor(io) +#else /* Ruby <3.1 */ static int my_fileno(VALUE io) { -#ifdef HAVE_RB_IO_DESCRIPTOR - if (TYPE(io) != T_FILE) - io = rb_convert_type(io, T_FILE, "IO", "to_io"); - - return rb_io_descriptor(io); -#else rb_io_t *fptr; - if (TYPE(io) != T_FILE) - io = rb_convert_type(io, T_FILE, "IO", "to_io"); GetOpenFile(io, fptr); if (fptr->fd < 0) rb_raise(rb_eIOError, "closed stream"); return fptr->fd; -#endif } +#endif /* Ruby <3.1 !HAVE_RB_IO_DESCRIPTOR */ diff --git a/ext/raindrops/tcp_info.c b/ext/raindrops/tcp_info.c index b82f705..c0d34e0 100644 --- a/ext/raindrops/tcp_info.c +++ b/ext/raindrops/tcp_info.c @@ -76,7 +76,7 @@ static VALUE alloc(VALUE klass) */ static VALUE init(VALUE self, VALUE io) { - int fd = my_fileno(io); + int fd = my_fileno(rb_io_get_io(io)); struct tcp_info *info = DATA_PTR(self); socklen_t len = (socklen_t)sizeof(struct tcp_info); int rc = getsockopt(fd, IPPROTO_TCP, TCP_INFO, info, &len);
alloca use was dangerous for architectures with >4K pages, and we can avoid some Ruby #to_io dispatches in some places. Eric Wong (4): avoid unnecessary #to_io calls my_fileno: use rb_io_check_closed for Ruby <3.1 linux_inet_diag: get rid of alloca usage test_linux_reuseport_tcp_listen_stats: skip w/o unicorn ext/raindrops/linux_inet_diag.c | 23 +++++++++++-------- ext/raindrops/my_fileno.h | 16 ++++--------- ext/raindrops/tcp_info.c | 2 +- test/test_linux_reuseport_tcp_listen_stats.rb | 2 +- 4 files changed, 20 insertions(+), 23 deletions(-)
> the "why" I think the missing piece here is that this change is necessary for compatibility with the current ruby master: https://bugs.ruby-lang.org/issues/19057#note-17 kgio seem to have the same code and fail in the same way.
Eric, it’s part of CRuby’s public headers since 3.1 was released.
Introduced in 4b8903421828cb9d4de139180563ae8d8f04e1ab
Hope that helps.
Warm regards,
Samuel
> On 9/06/2023, at 7:16 PM, Eric Wong <e@80x24.org> wrote:
>
> In the case of this patch; I would also like to know which Ruby
> implementation(s) and version(s) it appears in; and evidence
> backing up that it's a supported public API we can depend on
> going forward.