jruby / jruby

JRuby, an implementation of Ruby on the JVM
https://www.jruby.org
Other
3.8k stars 923 forks source link

File::EXCL is not thread safe #827

Closed minad closed 11 years ago

minad commented 11 years ago

See https://github.com/rubyspec/rubyspec/pull/231 for a failing snippet.

minad commented 11 years ago

You can also use this snippet:

require 'fileutils'

def create_file(name, value)
  ::File.open(name, ::File::WRONLY | ::File::CREAT | ::File::EXCL) do |f|
    f.write(value)
  end
  true
rescue Errno::EEXIST
  false
end

def create_thread(name)
  Thread.new do
    2000.times do |i|
      if create_file("data/#{i}", name)
        raise 'Concurrent create' if File.read("data/#{i}") != name
      end
      Thread.pass if rand(100) >= 99
    end
  end
end

FileUtils.mkdir('data')

a = create_thread('a')
b = create_thread('b')
c = create_thread('c')
a.join
b.join
c.join

FileUtils.rm_rf('data')
headius commented 11 years ago

Superseded by #828.

headius commented 11 years ago

Oops...I thought this was related to locking. Reopening.

headius commented 11 years ago

The spec is flawed. The files are opened for exclusive access, written to and closed. At that point, there's no further exclusive access. The subsequent read can (and obviously does) occur after another thread has done the same exclusive open + write + close.

If you want to test this properly, the subsequent read of the file contents must also occur inside the File.open block, because otherwise there's nothing preventing another thread from coming in and overwriting the file before the read happens.

Filing as Invalid. Reopen if you are able to show an actual JRuby issue.

minad commented 11 years ago

@headius I think you misunderstood. The argument O_CREAT | O_EXCL ensures that a file is created exclusively. The open by the next thread should fail. The snippet tests atomic file creation. Please read the UNIX man page man 2 open.

So this is still an issue. It works correctly under MRI.

headius commented 11 years ago

Yes, but the file is opened and then closed before the subsequent read that fails. Once a file is closed, there's no exclusive access anymore.

minad commented 11 years ago

The issue is that the file is overwritten and this should not be allowed. O_EXCL | O_CREAT means exclusive or atomic create.

minad commented 11 years ago

The issue is not about exclusive access, but about atomic create.

headius commented 11 years ago

Oh, right right. I'm getting my signals crossed with the locking issue still. Reopening to examine again.

minad commented 11 years ago

Therefore I opened two issues :)

headius commented 11 years ago

Ok, I think I see the problem in JRuby. We are creating the file using the appropriate exclusive-create APIs, but we do not raise an error if an exclusive create was not possible. Should be an easy fix, and this is a good find.

minad commented 11 years ago

Cool, thank you!

minad commented 11 years ago

that was fast! great!