Permalink
Browse files

JRUBY-4937: Add RubyFile.getUmaskSafe() to cache/sync and avoid troun…

…cing umask

Signed-off-by: Hiro Asari <asari.ruby@gmail.com>
  • Loading branch information...
1 parent 5e3ddc7 commit a7f6213d7a1d737f847b09374378f66b8314ac5e @dekellum dekellum committed with BanzaiMan Jul 15, 2010
Showing with 30 additions and 4 deletions.
  1. +30 −4 src/org/jruby/RubyFile.java
View
34 src/org/jruby/RubyFile.java
@@ -94,6 +94,9 @@
private static final int FNM_CASEFOLD = 8;
private static final int FNM_SYSCASE;
+ private static int _cachedUmask = 0;
+ private static final Object _umaskLock = new Object();
+
static {
if (Platform.IS_WINDOWS) {
FNM_SYSCASE = FNM_CASEFOLD;
@@ -519,7 +522,8 @@ protected void sysopenInternal(String path, ModeFlags modes, int perm) throws In
openFile.setPath(path);
openFile.setMode(modes.getOpenFileFlags());
- int umask = getRuntime().getPosix().umask(0);
+
+ int umask = getUmaskSafe( getRuntime() );
perm = perm - (perm & umask);
ChannelDescriptor descriptor = sysopen(path, modes, perm);
@@ -1640,10 +1644,13 @@ public static IRubyObject umask(ThreadContext context, IRubyObject recv, IRubyOb
Ruby runtime = context.getRuntime();
int oldMask = 0;
if (args.length == 0) {
- oldMask = runtime.getPosix().umask(0);
- runtime.getPosix().umask(oldMask);
+ oldMask = getUmaskSafe( runtime );
} else if (args.length == 1) {
- oldMask = runtime.getPosix().umask((int) args[0].convertToInteger().getLongValue());
+ int newMask = (int) args[0].convertToInteger().getLongValue();
+ synchronized (_umaskLock) {
+ oldMask = runtime.getPosix().umask(newMask);
+ _cachedUmask = newMask;
+ }
} else {
runtime.newArgumentError("wrong number of arguments");
}
@@ -1748,6 +1755,25 @@ public static ZipEntry getDirOrFileEntry(ZipFile zf, String path) throws IOExcep
}
/**
+ * Joy of POSIX, only way to get the umask is to set the umask,
+ * then set it back. That's unsafe in a threaded program. We
+ * minimize but may not totally remove this race by caching the
+ * obtained or previously set (see umask() above) umask and using
+ * that as the initial set value which, cross fingers, is a
+ * no-op. The cache access is then synchronized. TODO: Better?
+ */
+ private static int getUmaskSafe( Ruby runtime ) {
+ synchronized (_umaskLock) {
+ final int umask = runtime.getPosix().umask(_cachedUmask);
+ if (_cachedUmask != umask ) {
+ runtime.getPosix().umask(umask);
+ _cachedUmask = umask;
+ }
+ return umask;
+ }
+ }
+
+ /**
* Extract a timeval (an array of 2 longs: seconds and microseconds from epoch) from
* an IRubyObject.
*/

0 comments on commit a7f6213

Please sign in to comment.