Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JRuby raises IOError instead of specific error if File.rename(a,b) fails due to hidden filesystem #5318

Closed
bgalbrecht opened this Issue Sep 21, 2018 · 8 comments

Comments

Projects
None yet
3 participants
@bgalbrecht
Copy link

bgalbrecht commented Sep 21, 2018

Environment

jruby -v
jruby 9.2.0.0 (2.5.0) 2018-05-24 81156a8 OpenJDK 64-Bit Server VM 25.172-b11 on 1.8.0_172-b11 +jit [freebsd-x86_64]
FreeBSD jrails5.zuhause.org 11.1-RELEASE-p9 FreeBSD 11.1-RELEASE-p9 #0: Tue Apr  3 16:59:16 UTC 2018     root@amd64-builder.daemonology.net:/usr/obj/usr/src/sys/GENERIC  amd64

This is running in a FreeBSD jail where /tmp is a tmpfs file system, current path is in /home, which is under the root filesystem, and enforce.statfs=2 flag is set for the jail, so Files.getFileStore fails with Mount point not found in fstab if one or both paths are within /tmp. This difference causes bundler to fail.

Expected Behavior

jrails5% ls -ld /tmp/junk1
dr-xr-xr-x  2 bruce  wheel  0 Sep 20 23:49 /tmp/junk1/
jrails5% rbenv version
2.5.1 (set by /home/bruce/.ruby-version)
jrails5% irb
irb(main):001:0> File.rename('/tmp/zxc', 'asdx')
Traceback (most recent call last):
        3: from /home/bruce/.rbenv/versions/2.5.1/bin/irb:11:in `<main>'
        2: from (irb):1
        1: from (irb):1:in `rename'
Errno::EXDEV (Cross-device link @ rb_file_s_rename - (/tmp/zxc, asdx))
irb(main):002:0> File.rename('/tmp/zxc','/tmp/junk1/asd')
Traceback (most recent call last):
        3: from /home/bruce/.rbenv/versions/2.5.1/bin/irb:11:in `<main>'
        2: from (irb):2
        1: from (irb):2:in `rename'
Errno::EACCES (Permission denied @ rb_file_s_rename - (/tmp/zxc, /tmp/junk1/asd))
irb(main):003:0> 

Actual Behavior

jrails5% irb
irb(main):001:0> File.rename('/tmp/zxc', 'asdx')
Traceback (most recent call last):
        7: from /home/bruce/.rbenv/versions/jruby-9.2.0.0/bin/irb:13:in `<main>'
        6: from org/jruby/RubyKernel.java:1180:in `catch'
        5: from org/jruby/RubyKernel.java:1180:in `catch'
        4: from org/jruby/RubyKernel.java:1418:in `loop'
        3: from org/jruby/RubyKernel.java:1037:in `eval'
        2: from (irb):1:in `<eval>'
        1: from org/jruby/RubyFile.java:1097:in `rename'
IOError (Mount point not found in fstab)
irb(main):002:0> File.rename('/tmp/zxc','/tmp/junk1/asd')
Traceback (most recent call last):
        7: from /home/bruce/.rbenv/versions/jruby-9.2.0.0/bin/irb:13:in `<main>'
        6: from org/jruby/RubyKernel.java:1180:in `catch'
        5: from org/jruby/RubyKernel.java:1180:in `catch'
        4: from org/jruby/RubyKernel.java:1418:in `loop'
        3: from org/jruby/RubyKernel.java:1037:in `eval'
        2: from (irb):2:in `<eval>'
        1: from org/jruby/RubyFile.java:1097:in `rename'
IOError (Mount point not found in fstab)
@headius

This comment has been minimized.

Copy link
Member

headius commented Sep 21, 2018

While @bgalbrecht was filing this I poked around a bit and found the cause.

This "Mount point not found in fstab" error is actually from OpenJDK code used by the Paths subsystem to get the filesystem for a given Path.

The logic in OpenJDK differs from that in CRuby in that while the latter does a statfs and uses the results therein to compare filesystems, the former gets a list of all mounted filesystems and iterates over them to find one with a device matching the Path in hand. If it does not find that device listed as a mounted filesystem, it throws this lovely error as a dumb IOException that we turn into a dumb IOError.

It happens in this case because @bgalbrecht is running in a FreeBSD jail, which apparently does not expose the mount point of /tmp to jailed processes, and therefore OpenJDK refuses to return any filesystem information for any jailed /tmp file.

The logic of rename(2) on BSD comes into play here, since it appears that's what CRuby calls. The man page just says this about how EXDEV might be raised:

The link named by new and the file named by old are on different logical devices (file systems). Note
that this error code will not be returned if the implementation permits cross-device links.

This says nothing about files on the same filesystem that doesn't have an fstab entry, but @bgalbrecht's original output shows that CRuby is unable to rename a file from a location in /tmp to a location in /tmp, so it appears that BSD rename has similar behavior to OpenJDK here.

There are two options for fixing this:

  • Call native rename when on BSD. This would be simple enough but we'd still want the non-native (OpenJDK) fallback since it appears to work on other platforms (as far as we know). It would require an update to jnr-posix to add this function just for *BSD and Darwin.
  • Check the exception for this exact text and document that it's a gross hack due to a gross exception from OpenJDK. We'll likely need to do this anyway since at best we'd only be adding a native rename for *BSD+MacOS and need to continue to operate without native access.
@headius

This comment has been minimized.

Copy link
Member

headius commented Sep 21, 2018

@enebo In terms of 9.2.1, the message-checking change is very low-risk and just affects this one error condition.

@headius headius changed the title JRuby 9.2 returns different error from MRI if File.rename(a,b) is on a filesystem and Files.getFileStore fails with Mount point not found in fstab JRuby raises IOError instead of specific error if File.rename(a,b) fails due to hidden filesystem Sep 21, 2018

@headius

This comment has been minimized.

Copy link
Member

headius commented Sep 21, 2018

Awesome...just figured out that nio.2 does use native rename but if there's any error it just returns false. That's why we proceed to inspect the filesystems and then get this heinous error.

@headius

This comment has been minimized.

Copy link
Member

headius commented Sep 21, 2018

It appears the ATOMIC_MOVE operation on java.nio.file.Files.move might be the behavior of native rename we hope to emulate: https://docs.oracle.com/javase/9/docs/api/java/nio/file/Files.html#move-java.nio.file.Path-java.nio.file.Path-java.nio.file.CopyOption...-

This would be preferable to adding and depending on another native function binding.

@headius headius added this to the JRuby 9.2.1.0 milestone Sep 21, 2018

@bgalbrecht

This comment has been minimized.

Copy link
Author

bgalbrecht commented Sep 21, 2018

Since I first observed this when bundler failed when trying to install rails in this jail, if ATOMIC_MOVE fails in this case, it needs to throw an error that bundler expects.

@enebo

This comment has been minimized.

Copy link
Member

enebo commented Sep 21, 2018

@headius it wouldn't be the first time we had to examine text. I have never thought about this until now but I half wonder if we work with other localized error texts when we do this? I guess it depends on what we specifically match against.

headius added a commit to headius/jruby that referenced this issue Oct 11, 2018

Use JDK atomic move to more closely emulate POSIX rename.
Fixes jruby#5318.

Note that this no longer does the explicit file store check,
because that failed in an unrelated way when running under a
FreeBSD jail. Instead this now relies on the Files subsystem of
JDK to percolate out the original error message, which we should
detect properly and turn into EXDEV.
@headius

This comment has been minimized.

Copy link
Member

headius commented Oct 11, 2018

@bgalbrecht I've pushed a work-in-progress PR in #5356 that may work properly for you. I do not have access to a FreeBSD system with any jails set up...can you try to test it?

@headius

This comment has been minimized.

Copy link
Member

headius commented Oct 11, 2018

The PR looks good. @bgalbrecht If you can verify that this works correctly, we'll merge it for 9.2.1.

headius added a commit that referenced this issue Oct 31, 2018

headius added a commit that referenced this issue Oct 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.