-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
IO#fsync doesn't trigger an fsync(2) syscall? #4073
Comments
Well let's see what our fsync does. It is most likely just calling JDK classes, which may sync in a different way... |
So on master, it appears that fsync is implemented to only do the flush to OS, not the subsequent OS sync. That could indeed be a bug. It appears part of the MRI code I ported did not make it over. Interestingly, though, I see that 1.7 also does not make any call which would eventually sync at the OS level. So if this is a bug, it's an undiscovered bug for quite some time. |
Ok, I'm rigging a patch that will actually fsync anything that has a FileChannel and anything in a NativeSelectableChannel from jnr-enxio. I don't know how to do the same for e.g. JDK SocketChannel, pipes, and other non-file streams. |
Sounds good to me, thanks @headius :) |
Hmm...my patch seems like it should be doing the right thing, but I'm not sure if it's working. Can you test it out? diff --git a/core/src/main/java/org/jruby/RubyIO.java b/core/src/main/java/org/jruby/RubyIO.java
index 1f54e7e..aaa2bb9 100644
--- a/core/src/main/java/org/jruby/RubyIO.java
+++ b/core/src/main/java/org/jruby/RubyIO.java
@@ -1754,10 +1754,18 @@ public class RubyIO extends RubyObject implements IOEncodable {
if (fptr.io_fflush(context) < 0)
throw runtime.newSystemCallError("");
-// # ifndef _WIN32 /* already called in io_fflush() */
-// if ((int)rb_thread_io_blocking_region(nogvl_fsync, fptr, fptr->fd) < 0)
-// rb_sys_fail_path(fptr->pathv);
-// # endif
+ if (!Platform.IS_WINDOWS) { /* already called in io_fflush() */
+ try {
+ if (fptr.fileChannel() != null) fptr.fileChannel().force(true);
+ if (fptr.fd().chNative != null) {
+ int ret = runtime.getPosix().fsync(fptr.fd().chNative.getFD());
+ if (ret < 0) throw runtime.newErrnoFromInt(runtime.getPosix().errno());
+ }
+ } catch (IOException ioe) {
+ throw runtime.newIOErrorFromException(ioe);
+ }
+ }
+
return RubyFixnum.zero(runtime);
}
|
Yep! I'm seeing the call now:
Unfortunately I don't have a full app to use to check it more thoroughly, but at least it matches MRI Ruby now :) Thanks again, |
Excellent, I'll pull the change in. Thanks! |
Fixed for 9.1.3.0 and 1.7.26. |
Hi there,
When I make a call to IO#fsync on an open file handle, I expected to
see an fsync(2) syscall triggered (as MRI Ruby does), but in the
current JRuby master branch I don't see that.
I'm testing on
Linux thweeble 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt20-1+deb8u4 (2016-02-29) x86_64 GNU/Linux
as follows:Does this seem like a potential bug? I couldn't find any other issues
referencing this.
Thanks!
Mark
The text was updated successfully, but these errors were encountered: