From c8626bec8eb8e645d7d8b4add57c506dc7c6dd4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Sat, 5 Nov 2022 21:48:08 +0100 Subject: [PATCH 1/2] Implement `flock_*` fiber-aware, without blocking the thread --- spec/std/file_spec.cr | 38 +++++++++++++++++++++++++++++++-- src/crystal/system/unix/file.cr | 25 +++++++++++++++++----- src/io/file_descriptor.cr | 1 - 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/spec/std/file_spec.cr b/spec/std/file_spec.cr index 54993835500d..9c577f231b7e 100644 --- a/spec/std/file_spec.cr +++ b/spec/std/file_spec.cr @@ -940,10 +940,10 @@ describe "File" do File.open(datapath("test_file.txt")) do |file1| File.open(datapath("test_file.txt")) do |file2| file1.flock_exclusive do - # BUG: check for EWOULDBLOCK when exception filters are implemented - expect_raises(IO::Error, "Error applying or removing file lock") do + exc = expect_raises(IO::Error, "Error applying file lock: file is already locked") do file2.flock_exclusive(blocking: false) { } end + exc.os_error.should eq Errno::EWOULDBLOCK end end end @@ -958,6 +958,40 @@ describe "File" do end end end + + it "#flock_shared non-blocking" do + File.open(datapath("test_file.txt")) do |file1| + File.open(datapath("test_file.txt")) do |file2| + done = Channel(Nil).new + file1.flock_exclusive + + spawn do + file1.flock_unlock + done.send nil + end + + file2.flock_shared + done.receive + end + end + end + + it "#flock_exclusive non-blocking" do + File.open(datapath("test_file.txt")) do |file1| + File.open(datapath("test_file.txt")) do |file2| + done = Channel(Nil).new + file1.flock_exclusive + + spawn do + file1.flock_unlock + done.send nil + end + + file2.flock_exclusive + done.receive + end + end + end end it "reads at offset" do diff --git a/src/crystal/system/unix/file.cr b/src/crystal/system/unix/file.cr index a73a190f45a5..87ff554a3b99 100644 --- a/src/crystal/system/unix/file.cr +++ b/src/crystal/system/unix/file.cr @@ -252,14 +252,29 @@ module Crystal::System::File flock LibC::FlockOp::UN end - private def flock(op : LibC::FlockOp, blocking : Bool = true) - op |= LibC::FlockOp::NB unless blocking + private def flock(op : LibC::FlockOp, retry : Bool) : Nil + op |= LibC::FlockOp::NB - if LibC.flock(fd, op) != 0 - raise IO::Error.from_errno("Error applying or removing file lock") + if retry + until flock(op) + ::Fiber.yield + end + else + flock(op) || raise IO::Error.from_errno("Error applying file lock: file is already locked") end + end - nil + private def flock(op) : Bool + if 0 == LibC.flock(fd, op) + true + else + errno = Errno.value + if errno.in?(Errno::EAGAIN, Errno::EWOULDBLOCK) + false + else + raise IO::Error.from_os_error("Error applying or removing file lock", errno) + end + end end private def system_fsync(flush_metadata = true) : Nil diff --git a/src/io/file_descriptor.cr b/src/io/file_descriptor.cr index 6da5b859f77b..ecbcc6cebf88 100644 --- a/src/io/file_descriptor.cr +++ b/src/io/file_descriptor.cr @@ -175,7 +175,6 @@ class IO::FileDescriptor < IO end # TODO: use fcntl/lockf instead of flock (which doesn't lock over NFS) - # TODO: always use non-blocking locks, yield fiber until resource becomes available def flock_shared(blocking = true) flock_shared blocking From 5b34a25d14b66d03eeabfc98491263c00313d786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Fri, 18 Nov 2022 14:28:06 +0100 Subject: [PATCH 2/2] Reword tests --- spec/std/file_spec.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/std/file_spec.cr b/spec/std/file_spec.cr index 9c577f231b7e..74dc5007c415 100644 --- a/spec/std/file_spec.cr +++ b/spec/std/file_spec.cr @@ -959,7 +959,7 @@ describe "File" do end end - it "#flock_shared non-blocking" do + pending_win32 "#flock_shared soft blocking fiber" do File.open(datapath("test_file.txt")) do |file1| File.open(datapath("test_file.txt")) do |file2| done = Channel(Nil).new @@ -976,7 +976,7 @@ describe "File" do end end - it "#flock_exclusive non-blocking" do + pending_win32 "#flock_exclusive soft blocking fiber" do File.open(datapath("test_file.txt")) do |file1| File.open(datapath("test_file.txt")) do |file2| done = Channel(Nil).new