From 6c915760bb18a955a74d2ef2872a4366f127b4ef Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Sat, 19 Feb 2022 01:23:00 +0900 Subject: [PATCH 1/5] test_out_forward: Use assert_rr For avoiding the following error on Ruby-3.1 on Windows: ``` 2022-02-18T01:36:00.3691879Z =============================================================================== 2022-02-18T01:36:00.3692453Z Failure: test: send tags in str (utf-8 strings)(ForwardOutputTest): 2022-02-18T01:36:00.3692799Z On subject , 2022-02-18T01:36:00.3693302Z unexpected method invocation: 2022-02-18T01:36:00.3693595Z process_wait(2088, 0) 2022-02-18T01:36:00.3702794Z expected invocations: 2022-02-18T01:36:00.3703495Z C:/hostedtoolcache/windows/Ruby/3.1.0/x64/lib/ruby/gems/3.1.0/gems/serverengine-2.2.5/lib/serverengine/socket_manager.rb:79:in ``' 2022-02-18T01:36:00.3704338Z C:/hostedtoolcache/windows/Ruby/3.1.0/x64/lib/ruby/gems/3.1.0/gems/serverengine-2.2.5/lib/serverengine/socket_manager.rb:79:in `block in generate_path' 2022-02-18T01:36:00.3705025Z C:/hostedtoolcache/windows/Ruby/3.1.0/x64/lib/ruby/gems/3.1.0/gems/serverengine-2.2.5/lib/serverengine/socket_manager.rb:78:in `each' 2022-02-18T01:36:00.3705695Z C:/hostedtoolcache/windows/Ruby/3.1.0/x64/lib/ruby/gems/3.1.0/gems/serverengine-2.2.5/lib/serverengine/socket_manager.rb:78:in `generate_path' 2022-02-18T01:36:00.3706156Z D:/a/fluentd/fluentd/lib/fluent/test/driver/base.rb:105:in `instance_start' 2022-02-18T01:36:00.3706550Z D:/a/fluentd/fluentd/lib/fluent/test/driver/base.rb:77:in `run' 2022-02-18T01:36:00.3706945Z D:/a/fluentd/fluentd/lib/fluent/test/driver/base_owner.rb:130:in `run' 2022-02-18T01:36:00.3707422Z D:/a/fluentd/fluentd/test/plugin/test_out_forward.rb:434:in `block in ' 2022-02-18T01:36:00.3707780Z 431: ] 2022-02-18T01:36:00.3708021Z 432: 2022-02-18T01:36:00.3708366Z 433: stub(d.instance.ack_handler).read_ack_from_sock(anything).never 2022-02-18T01:36:00.3708758Z => 434: target_input_driver.run(expect_records: 2) do 2022-02-18T01:36:00.3709053Z 435: d.run do 2022-02-18T01:36:00.3709364Z 436: emit_events.each do |tag, t, record| 2022-02-18T01:36:00.3709681Z 437: d.feed(tag, t, record) 2022-02-18T01:36:00.3709997Z =============================================================================== ``` Signed-off-by: Takuro Ashie --- test/plugin/test_out_forward.rb | 50 ++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/test/plugin/test_out_forward.rb b/test/plugin/test_out_forward.rb index 53aca518c2..66a2776cc5 100644 --- a/test/plugin/test_out_forward.rb +++ b/test/plugin/test_out_forward.rb @@ -431,10 +431,12 @@ def try_write(chunk) ] stub(d.instance.ack_handler).read_ack_from_sock(anything).never - target_input_driver.run(expect_records: 2) do - d.run do - emit_events.each do |tag, t, record| - d.feed(tag, t, record) + assert_rr do + target_input_driver.run(expect_records: 2) do + d.run do + emit_events.each do |tag, t, record| + d.feed(tag, t, record) + end end end end @@ -461,10 +463,12 @@ def try_write(chunk) ] stub(d.instance.ack_handler).read_ack_from_sock(anything).never - target_input_driver.run(expect_records: 2) do - d.run(default_tag: 'test') do - records.each do |record| - d.feed(time, record) + assert_rr do + target_input_driver.run(expect_records: 2) do + d.run(default_tag: 'test') do + records.each do |record| + d.feed(time, record) + end end end end @@ -491,10 +495,12 @@ def try_write(chunk) {"a" => 2} ] stub(d.instance.ack_handler).read_ack_from_sock(anything).never - target_input_driver.run(expect_records: 2) do - d.run(default_tag: 'test') do - records.each do |record| - d.feed(time, record) + assert_rr do + target_input_driver.run(expect_records: 2) do + d.run(default_tag: 'test') do + records.each do |record| + d.feed(time, record) + end end end end @@ -549,10 +555,12 @@ def try_write(chunk) ] # not attempt to receive responses stub(d.instance.ack_handler).read_ack_from_sock(anything).never - target_input_driver.run(expect_records: 2) do - d.run(default_tag: 'test') do - records.each do |record| - d.feed(time, record) + assert_rr do + target_input_driver.run(expect_records: 2) do + d.run(default_tag: 'test') do + records.each do |record| + d.feed(time, record) + end end end end @@ -575,10 +583,12 @@ def try_write(chunk) ] # not attempt to receive responses stub(d.instance.ack_handler).read_ack_from_sock(anything).never - target_input_driver.run(expect_records: 2) do - d.run(default_tag: 'test') do - records.each do |record| - d.feed(time, record) + assert_rr do + target_input_driver.run(expect_records: 2) do + d.run(default_tag: 'test') do + records.each do |record| + d.feed(time, record) + end end end end From 3e0688f9ba92a8b964c33ba9cba3cd97ec86efd9 Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Mon, 30 May 2022 22:11:17 +0900 Subject: [PATCH 2/5] Fix broken Fluent::FileWrapper on Ruby 3.1 on UCRT Although we have used WindowsFile as altenertive of File mainly for specifying FILE_SHARE_DELETE to CreateFile, WindowsFile#io always raises Errno::EBADF on Ruby 3.1 on UCRT. Instead we can use File.open with File::Constants::SHARE_DELETE to avoid it. It was introduced for Fluentd, so we should use it :D https://bugs.ruby-lang.org/issues/11218 Signed-off-by: Takuro Ashie --- lib/fluent/plugin/file_wrapper.rb | 41 +++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/lib/fluent/plugin/file_wrapper.rb b/lib/fluent/plugin/file_wrapper.rb index 57d6b4949d..14a3f5e154 100644 --- a/lib/fluent/plugin/file_wrapper.rb +++ b/lib/fluent/plugin/file_wrapper.rb @@ -16,8 +16,45 @@ module Fluent module FileWrapper - def self.open(*args) - io = WindowsFile.new(*args).io + include File::Constants + RUBY_3_1_OR_LATER = Gem::Version.create(RUBY_VERSION) >= Gem::Version.create('3.1.0') + + def self.mode2flags(mode) + # Always need BINARY to enable SHARE_DELETE + # https://bugs.ruby-lang.org/issues/11218 + # https://github.com/ruby/ruby/blob/d6684f063bc53e3cab025bd39526eca3b480b5e7/win32/win32.c#L6332-L6345 + flags = BINARY | SHARE_DELETE + case mode.delete("b") + when "r" + flags |= RDONLY + when "r+" + flags |= RDWR + when "w" + flags |= WRONLY | CREAT | TRUNC + when "w+" + flags |= RDWR | CREAT | TRUNC + when "a" + flags |= WRONLY | CREAT | APPEND + when "a+" + flags |= RDWR | CREAT | APPEND + else + raise Errno::EINVAL.new("Unsupported mode by Fluent::FileWrapper: #{mode}") + end + end + + def self.open(path, mode='r') + if RUBY_3_1_OR_LATER + # On Ruby 3.1 with UCRT, WindowsFile causes EBADF on calling File.for_fd + # at WindowsFile#io. So use File.open with SHARE_DELETE instead. + # https://github.com/fluent/fluentd/pull/3585#issuecomment-1101502617 + io = File.open(path, mode2flags(mode)) + else + # Although File.open supports SHARE_DELETE since Ruby 2.3, we keep using + # WindowsFile for Ruby 3.0 or former to make sure to keep backward + # compatibility. + io = WindowsFile.new(path, mode).io + end + if block_given? v = yield io io.close From cde1391aaaa6c5eae04895428aca7431eab4f3e1 Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Mon, 30 May 2022 22:27:55 +0900 Subject: [PATCH 3/5] GitHub Actions: Add Ruby 3.1 & head for Windows Signed-off-by: Takuro Ashie --- .github/workflows/windows-test.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/windows-test.yaml b/.github/workflows/windows-test.yaml index 1f43f18e57..12a7537c57 100644 --- a/.github/workflows/windows-test.yaml +++ b/.github/workflows/windows-test.yaml @@ -13,11 +13,14 @@ jobs: strategy: fail-fast: false matrix: - ruby-version: ['2.7', '2.6'] + ruby-version: ['3.1', '2.7'] os: - windows-latest experimental: [false] include: + - ruby-version: head + os: windows-latest + experimental: true - ruby-version: '3.0.3' os: windows-latest experimental: false From 71f6fb3cca8efefc22e8468a79abf4a44e0afaf8 Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Tue, 31 May 2022 10:28:23 +0900 Subject: [PATCH 4/5] FileWrapper: Use File.open also on older Ruby It seems work fine than WindowsFile, so remove old code to simplify the code. Signed-off-by: Takuro Ashie --- lib/fluent/plugin/file_wrapper.rb | 42 +++++-------------------------- 1 file changed, 6 insertions(+), 36 deletions(-) diff --git a/lib/fluent/plugin/file_wrapper.rb b/lib/fluent/plugin/file_wrapper.rb index 14a3f5e154..621cb259d9 100644 --- a/lib/fluent/plugin/file_wrapper.rb +++ b/lib/fluent/plugin/file_wrapper.rb @@ -17,8 +17,6 @@ module Fluent module FileWrapper include File::Constants - RUBY_3_1_OR_LATER = Gem::Version.create(RUBY_VERSION) >= Gem::Version.create('3.1.0') - def self.mode2flags(mode) # Always need BINARY to enable SHARE_DELETE # https://bugs.ruby-lang.org/issues/11218 @@ -43,18 +41,9 @@ def self.mode2flags(mode) end def self.open(path, mode='r') - if RUBY_3_1_OR_LATER - # On Ruby 3.1 with UCRT, WindowsFile causes EBADF on calling File.for_fd - # at WindowsFile#io. So use File.open with SHARE_DELETE instead. - # https://github.com/fluent/fluentd/pull/3585#issuecomment-1101502617 - io = File.open(path, mode2flags(mode)) - else - # Although File.open supports SHARE_DELETE since Ruby 2.3, we keep using - # WindowsFile for Ruby 3.0 or former to make sure to keep backward - # compatibility. - io = WindowsFile.new(path, mode).io - end - + # inject File::Constants::SHARE_DELETE + # https://github.com/fluent/fluentd/pull/3585#issuecomment-1101502617 + io = File.open(path, mode2flags(mode)) if block_given? v = yield io io.close @@ -72,17 +61,6 @@ def self.stat(path) end end - module WindowsFileExtension - attr_reader :path - - def stat - s = super - s.instance_variable_set :@ino, @ino - def s.ino; @ino; end - s - end - end - class Win32Error < StandardError require 'windows/error' include Windows::Error @@ -125,7 +103,9 @@ def wsaerr? end end - # To open and get stat with setting FILE_SHARE_DELETE + # To open and get stat with setting FILE_SHARE_DELETE. + # Although recent Ruby's File.stat uses it, we still need this to keep + # backward compatibility of ino and delete_pending methods. class WindowsFile require 'windows/file' require 'windows/error' @@ -172,16 +152,6 @@ def close @file_handle = INVALID_HANDLE_VALUE end - def io - fd = _open_osfhandle(@file_handle, 0) - raise Errno::ENOENT if fd == -1 - io = File.for_fd(fd, @mode) - io.instance_variable_set :@ino, self.ino - io.instance_variable_set :@path, @path - io.extend WindowsFileExtension - io - end - def ino by_handle_file_information = '\0'*(4+8+8+8+4+4+4+4+4+4) #72bytes From c471cfcf4e34fc177b562270ef8333e8e185e54a Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Tue, 31 May 2022 16:43:10 +0900 Subject: [PATCH 5/5] Add a missing blank line Signed-off-by: Takuro Ashie --- lib/fluent/plugin/file_wrapper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/fluent/plugin/file_wrapper.rb b/lib/fluent/plugin/file_wrapper.rb index 621cb259d9..f05af89205 100644 --- a/lib/fluent/plugin/file_wrapper.rb +++ b/lib/fluent/plugin/file_wrapper.rb @@ -17,6 +17,7 @@ module Fluent module FileWrapper include File::Constants + def self.mode2flags(mode) # Always need BINARY to enable SHARE_DELETE # https://bugs.ruby-lang.org/issues/11218