[cp-patches] FYI: FileLock fixes

[Mark Wielaard]

Hi,

I created some mauve tests and found some bugs with FileLock. The
biggest issue was that when we use LONG.MAX_VALUE, which means
everything from the given position, fcntl actually expects zero as size
argument for the same semantics.

2005-07-31  Mark Wielaard  <address@hidden>

       * gnu/java/nio/FileLockImpl.java: Mark class final.
       (ch): Removed field.
       (valid): New field.
       (FileLockImpl): Set valid to true.
       (isValid): Test and set valid field.
       (release): Release lock if it is still valid.
       * java/nio/channels/FileLock.java: Mark all fields private final.
       * native/jni/java-nio/gnu_java_nio_channels_FileChannelImpl.c
       (Java_gnu_java_nio_channels_FileChannelImpl_lock): Translate
       Long.MAX_VALUE to zero for fcntl.
       (Java_gnu_java_nio_channels_FileChannelImpl_unlock): Likewise.

This also finally makes it possible to run eclipse without needing to
give it the -Dosgi.locking=none option.

Committed,

Mark

Index: gnu/java/nio/FileLockImpl.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/nio/FileLockImpl.java,v
retrieving revision 1.9
diff -u -r1.9 FileLockImpl.java
--- gnu/java/nio/FileLockImpl.java      2 Jul 2005 20:32:13 -0000       1.9
+++ gnu/java/nio/FileLockImpl.java      31 Jul 2005 15:37:11 -0000
@@ -1,5 +1,5 @@
-/* FileLockImpl.java -- 
-   Copyright (C) 2002, 2004 Free Software Foundation, Inc.
+/* FileLockImpl.java -- FileLock associated with a FileChannelImpl.
+   Copyright (C) 2002, 2004, 2005 Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -44,20 +44,29 @@
 import java.nio.channels.FileLock;
 
 /**
+ * A FileLock associated with a FileChannelImpl.
+ *
  * @author Michael Koch
  * @since 1.4
  */
-public class FileLockImpl extends FileLock
+public final class FileLockImpl extends FileLock
 {
-  private FileChannelImpl ch;
-  
+  /**
+   * Whether or not this lock is valid, false when channel is closed or
+   * release has been explicitly called.
+   */
+  private boolean valid;
+
   public FileLockImpl (FileChannelImpl channel, long position,
                        long size, boolean shared)
   {
     super (channel, position, size, shared);
-    ch = channel;
+    valid = true;
   }
 
+  /**
+   * Releases this lock.
+   */
   protected void finalize()
   {
     try
@@ -70,13 +79,26 @@
       }
   }
   
-  public boolean isValid ()
+  /**
+   * Whether or not this lock is valid, false when channel is closed or
+   * release has been explicitly called.
+   */
+  public boolean isValid()
   {
-    return channel().isOpen();
+    if (valid)
+      valid = channel().isOpen();
+    return valid;
   }
 
-  public synchronized void release () throws IOException
+  /**
+   * Releases the lock if it is still valid. Marks this lock as invalid.
+   */
+  public void release() throws IOException
   {
-    ch.unlock(position(), size());
+    if (isValid())
+      {
+       valid = false;
+       ((FileChannelImpl) channel()).unlock(position(), size());
+      }
   }
 }
Index: java/nio/channels/FileLock.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/nio/channels/FileLock.java,v
retrieving revision 1.11
diff -u -r1.11 FileLock.java
--- java/nio/channels/FileLock.java     2 Jul 2005 20:32:39 -0000       1.11
+++ java/nio/channels/FileLock.java     31 Jul 2005 15:37:12 -0000
@@ -45,10 +45,10 @@
  */
 public abstract class FileLock
 {
-  FileChannel channel;
-  long position;
-  long size;
-  boolean shared;
+  private final FileChannel channel;
+  private final long position;
+  private final long size;
+  private final boolean shared;
 
   /**
    * Initializes the file lock.
Index: native/jni/java-nio/gnu_java_nio_channels_FileChannelImpl.c
===================================================================
RCS file: 
/cvsroot/classpath/classpath/native/jni/java-nio/gnu_java_nio_channels_FileChannelImpl.c,v
retrieving revision 1.23
diff -u -r1.23 gnu_java_nio_channels_FileChannelImpl.c
--- native/jni/java-nio/gnu_java_nio_channels_FileChannelImpl.c 30 Jul 2005 
20:40:30 -0000      1.23
+++ native/jni/java-nio/gnu_java_nio_channels_FileChannelImpl.c 31 Jul 2005 
15:37:12 -0000
@@ -851,17 +851,23 @@
   flock.l_type = shared ? F_RDLCK : F_WRLCK;
   flock.l_whence = SEEK_SET;
   flock.l_start = (off_t) position;
-  flock.l_len = (off_t) size;
+  /* Long.MAX_VALUE means lock everything possible starting at pos. */
+  if (size == 9223372036854775807LL)
+    flock.l_len = 0;
+  else
+    flock.l_len = (off_t) size;
 
   ret = fcntl (fd, cmd, &flock);
+  /* fprintf(stderr, "fd %d, wait %d, shared %d, ret %d, position %lld, size 
%lld, l_start %ld, l_len %ld\n", fd, wait, shared,ret, position, size, (long) 
flock.l_start, (long) flock.l_len); */
   if (ret)
     {
       /* Linux man pages for fcntl state that errno might be either
          EACCES or EAGAIN if we try F_SETLK, and another process has
-         an overlapping lock. */
+         an overlapping lock. We should not get an unexpected errno. */
       if (errno != EACCES && errno != EAGAIN)
         {
-          JCL_ThrowException (env, IO_EXCEPTION, strerror (errno));
+          JCL_ThrowException (env, "java/lang/InternalError",
+                             strerror (errno));
         }
       return JNI_FALSE;
     }
@@ -892,12 +898,17 @@
   flock.l_type = F_UNLCK;
   flock.l_whence = SEEK_SET;
   flock.l_start = (off_t) position;
-  flock.l_len = (off_t) length;
+  /* Long.MAX_VALUE means unlock everything possible starting at pos. */
+  if (length == 9223372036854775807LL)
+    flock.l_len = 0;
+  else
+    flock.l_len = (off_t) length;
 
   ret = fcntl (fd, F_SETLK, &flock);
   if (ret)
     {
-      JCL_ThrowException (env, IO_EXCEPTION, strerror (errno));
+      JCL_ThrowException (env, "java/lang/InternalError",
+                         strerror (errno));
     }
 #else
   (void) obj;

signature.asc
Description: This is a digitally signed message part