-
-
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
File.flock does not work on Solaris for 9k #3343
Comments
Hearing 9.0.2.0 coming out really soon. Any chance to squeak a fix for this issue? I just tried a snapshot from ci.jruby.com but problem is still present. It is a blocker for my to run 9k on Solaris, since I can't install gems |
Looking at the code, I see the same fix for #3254 applied in JRuby master:
The only way this check could pass is if we're not on Solaris, or if that check is not correctly detecting Solaris. |
ok... weird |
I'm REALLY confused. 1.7.22 responds the same way but I don't have the issue on 1.7.22 ./jruby --version ./jruby -e "File.open('/tmp/test', File::RDWR|File::CREAT) { |f| f.flock(File::LOCK_EX) }" ./jruby -e 'import org.jruby.platform.Platform; puts Platform::IS_SOLARIS' |
on irc, was asked for uname -a output |
I believe 1.7 did not have native flock logic, so it wouldn't hit the bug. It seems our IS_SOLARIS is faulty somehow. Will look into that. |
@oblutak Can you provide the output of the following code? puts ENV_JAVA["os.name"]
puts ENV_JAVA["os.arch"] The fact that io/console is giving you a warning makes me think we're not handling native code loading right on Solaris either. |
Ok, I have a fix for the IS_SOLARIS problem, but there's a larger issue. In JRuby 9k, we tend to prefer real native channels even for opening files. Unfortunately that means we can't use the pure-Java fallback logic here anyway. We'll either need to not use native file channels on Solaris or, more likely, emulate flock with fcntl the same way MRI does. |
Here is the requested info On your last comment about flock emulation, is that something that would be done in jnr-posix? Unfortunately, I'm not a coder so can't help that way, but if there is anything I could help testing with, please let me know. |
@oblutak Yes, I'm working on the flock emulation in jnr-posix. I've hit some snags, though, so this won't make it into the next 9k release. It should be in the one after that. |
Understood. Thanks for the update. Would you rather I open a different issue for the submission of the IS_SOLARIS fix, or will you just submit both fixes under this issue? |
@oblutak I'll do both fixes as part of this issue. |
Bumping to 9.0.5.0 because 9.0.4 will be another quick flip to fix some regressions. |
I never got back to this, but I did at least start to attempt a flock impl for Solaris based on fcntl. Unfortunately never completed and had to revert when I accidentally pushed it: jnr/jnr-posix@32b9c84 |
Bumping to 9.1 so we can get 9.0.5 out. |
@oblutak For 9.1 the current code basically just falls back on the old JDK logic when we're on Solaris. Can you test out a 9.1 snapshot build and let me know if it's working ok for you? http://ci.jruby.org |
I believe I found the bug in my patch and will attempt to apply it now. Downloading a Solaris vbox VM to test against. |
Pushed my tweaks for the patch in jnr/jnr-posix@f34106c (patch was applied in previous commit). |
Ok, I was unable to get the flock test to pass on Solaris and I have another task I need to work on. @oblutak If you're still out there, you could pull master from jnr-posix and try to get the flock test passing on Solaris. I'm guessing the struct layout is not quite right, but I'm not certain. |
Patch to apply when flock is working in jnr-posix: diff --git a/core/src/main/java/org/jruby/util/io/PosixShim.java b/core/src/main/java/org/jruby/util/io/PosixShim.java
index 1c122dc..7a4ff67 100644
--- a/core/src/main/java/org/jruby/util/io/PosixShim.java
+++ b/core/src/main/java/org/jruby/util/io/PosixShim.java
@@ -187,7 +187,7 @@ public class PosixShim {
int real_fd = fd.realFileno;
- if (real_fd != -1 && real_fd < FilenoUtil.FIRST_FAKE_FD && !Platform.IS_SOLARIS) {
+ if (real_fd != -1 && real_fd < FilenoUtil.FIRST_FAKE_FD) {
// we have a real fd and not on Solaris...try native flocking
// see jruby/jruby#3254 and jnr/jnr-posix#60
int result = posix.flock(real_fd, lockMode); |
My apologies, I've been away from all (j)ruby activities since xmas. As indicated before, I'm no designer/coder so I quickly fall out of my element in too technical discussion. I also tried to read/search re: fcntl and flock. The behavior of the C code above just didn't make sense to me. Comments in stackoverflow discussion I think give a hint.
|
You are so busted :) This was a really good comment, I think you're right on the money. I've since been looking at the Linux
and:
(That's with Given the unit test exists (and presumably doesn't fail) for systems with native
On the other hand:
I agree this seems hard to emulate with plain |
MRI supports some of these platforms that don't have flock...we really just need to do what they're doing. I'd thought we might just be able to use pure-Java flocking, but then we'd need to make sure we're not using any native descriptors...which kills a bunch of more valuable features. |
See also ruby/spec#255. |
If I understand the flock_spec from ruby/spec
The test is checking that a file with an exclusive lock appears locked to another process. As for making jnr/posix tests passed for a flock-fcntl bridge, I don't know if that is possible. All existing flock test are perfectly valid for flock. Maybe fcntl locking needs to exists separately with its own set of unit tests (in jnr/posix) and at 'higher' layer (jruby?) need to make the distinction of using/implement a flock-fcntl bridge when a ruby 'flock' is called. (sorry, I said before I'm no coder so don't know the correct terminology but hopefully you understand what I am trying to say) |
@oblutak Actually that makes a lot of sense. jnr-posix should just implement and test both flock and fcntl, and if flock is not available it doesn't run the tests, like any other non-universal feature. We'd mimic MRI's logic for using fcntl as flock from the JRuby side. So I think next steps would be to ensure that fcntl locking works with jnr-posix's fcntl today and write some jnr-posix tests for it. Then we look at how MRI emulates flock with fcntl, implement that in JRuby, and we're good? |
I think the flock (using fnctl) as implemented in jnr-posix master in SolarisPOSIX.java replicates what is done in MRI. Now for fcntl locking unit tests for jnr-posix, I wish I could help but I have no idea how. I don't know how you can 'launch' another process, required to try to get a lock on the same file. Maybe we just do the positive paths test for now just to validate that fcntl calls return successfully and rely on the flock_spec test at ruby/jruby to confirm 'flock' at (j)ruby layer behave as expected. If you think there is a way I could help, let me know how, since I don't know java at all and never used tools like maven. I am trying a bit on my own. I got a copy of jnr-posix, got jdk-1.8u92 and maven 3.3.9 installed. Not sure what mvn commands to use. Doing |
@oblutak Can you recommend a good free Solaris (or friends) that I could use to verify the fix on a VirtualBox VM? |
@oblutak RE: testing...I'm not sure either. Is there a standard way we can attempt to lock a file without introducing a lot of dependencies? If not, then I think the next best option would be to simply have the test launch another JVM with some prepared code that tries to lock and prints its success. If the jnr-posix tests fail (and especially if they crash) we should get that fixed. |
I think this is turning out to be a bit more work than we can spin in the next week, so we'll bump this to 9.1.4.0 and work with @oblutak to get jnr-posix up to scratch on Solaris. |
Hi @headius, Sorry for the delay. I've spent vacations totally 'unplugged'. As for 'free Solaris and friends' I've only used Solaris on real hardware. On another issue reported, someone commented similar issue on SmartOS. There wiki has a section 'SmartOS as a Sandboxed VirtualBox Guest', but I don't have any personal experience to share. |
I have just pushed f8da225 for #4162, which implements Please test a build from http://ci.jruby.org once there's an updated snapshot. |
Looks promising! |
In case someone stumble across my previous comment... Error is/was |
Interesting. Narrow that down and file it...we'll get Solaris 100% yet! |
Same issue reported for 3254. The code change fixed the 1.7 branch (1.7.22) but issue is still in 9k. I thought it would be in 9.0.1.0 since it was released after:
The text was updated successfully, but these errors were encountered: