summaryrefslogtreecommitdiff
path: root/test/ruby
diff options
context:
space:
mode:
authorJeremy Evans <code@jeremyevans.net>2024-09-12 10:04:10 -0700
committerGitHub <noreply@github.com>2024-09-12 10:04:10 -0700
commitf2919bd11c570fc5f5440d1f101be38f61e3d16b (patch)
treecc1922bc8a3296c6069650ec4c434da40ab2eb53 /test/ruby
parent15135030e5808d527325feaaaf04caeb1b44f8b5 (diff)
Add error checking to readdir, telldir, and closedir calls in dir.c
Raise SystemCallError exception when these functions return an error. This changes behavior for the following case (found by the tests): ```ruby dir1 = Dir.new('..') dir2 = Dir.for_fd(dir1.fileno) dir1.close dir2.close ``` The above code is basically broken, as `dir1.close` closed the file descriptor. The subsequent `dir2.close` call is undefined behavior. When run in isolation, it raises Errno::EBADF after the change, but if another thread opens a file descriptor between the `dir1.close` and `dir2.close` calls, the `dir2.close` call could close the file descriptor opened by the other thread. Raising an exception is much better in this case as it makes it obvious there is a bug in the code. For the readdir check, since the GVL has already been released, reacquire it rb_thread_call_with_gvl if an exception needs to be raised. Due to the number of closedir calls, this adds static close_dir_data and check_closedir functions. The close_dir_data takes a struct dir_data * and handles setting the dir entry to NULL regardless of failure. Fixes [Bug #20586] Co-authored-by: Nobuyoshi Nakada <nobu.nakada@gmail.com>
Notes
Notes: Merged: https://github.com/ruby/ruby/pull/11393 Merged-By: jeremyevans <code@jeremyevans.net>
Diffstat (limited to 'test/ruby')
-rw-r--r--test/ruby/test_dir.rb4
1 files changed, 3 insertions, 1 deletions
diff --git a/test/ruby/test_dir.rb b/test/ruby/test_dir.rb
index b86cf330bc..78371a096b 100644
--- a/test/ruby/test_dir.rb
+++ b/test/ruby/test_dir.rb
@@ -710,7 +710,9 @@ class TestDir < Test::Unit::TestCase
assert_equal(new_dir.chdir{Dir.pwd}, for_fd_dir.chdir{Dir.pwd})
ensure
new_dir&.close
- for_fd_dir&.close
+ if for_fd_dir
+ assert_raise(Errno::EBADF) { for_fd_dir.close }
+ end
end
else
assert_raise(NotImplementedError) { Dir.for_fd(0) }