Use Opaque for locking instead of byte[], improve reuse of base64 keys

core/src/main/java/org/dcache/nfs/util/Opaque.java

@@ -21,6 +21,7 @@
 
 import java.io.Serializable;
 import java.util.Arrays;
+import java.util.Base64;
 
 import com.google.common.io.BaseEncoding;
 
@@ -32,15 +33,33 @@ public class Opaque implements Serializable {
     private static final long serialVersionUID = 1532238396149112674L;
 
     private final byte[] _opaque;
+    private String base64 = null;
 
+    public static Opaque forBytes(byte[] bytes) {
+        return new Opaque(bytes.clone());
+    }
+
+    @Deprecated(forRemoval = true)
     public Opaque(byte[] opaque) {
         _opaque = opaque;
     }
 
+    @Deprecated(forRemoval = true)
     public byte[] getOpaque() {
         return _opaque;
     }
 
+    public byte[] asBytes() {
+        return _opaque.clone();
+    }
+
+    public String getBase64() {
+        if (base64 == null) {
+            base64 = Base64.getEncoder().withoutPadding().encodeToString(_opaque);
+        }
+        return base64;
+    }
+
     @Override
     public int hashCode() {
         return Arrays.hashCode(_opaque);
@@ -58,6 +77,12 @@ public boolean equals(Object o) {
         return Arrays.equals(_opaque, ((Opaque) o)._opaque);
     }
 
+    /**
+     * Returns a (potentially non-stable) debug string.
+     * 
+     * @see #getBase64()
+     */
+    @Deprecated
     @Override
     public String toString() {
         StringBuilder sb = new StringBuilder();

core/src/main/java/org/dcache/nfs/v4/FileTracker.java

@@ -242,7 +242,7 @@ public OpenRecord addOpen(NFS4Client client, StateOwner owner, Inode inode, int
         // client explicitly requested write delegation
         boolean wantWriteDelegation = (shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG) != 0;
 
-        Opaque fileId = new Opaque(inode.getFileId());
+        Opaque fileId = inode.getFileIdKey();
         Lock lock = filesLock.get(fileId);
         lock.lock();
         try {
@@ -376,7 +376,7 @@ public OpenRecord addOpen(NFS4Client client, StateOwner owner, Inode inode, int
     public stateid4 downgradeOpen(NFS4Client client, stateid4 stateid, Inode inode, int shareAccess, int shareDeny)
             throws ChimeraNFSException {
 
-        Opaque fileId = new Opaque(inode.getFileId());
+        Opaque fileId = inode.getFileIdKey();
         Lock lock = filesLock.get(fileId);
         lock.lock();
         try {
@@ -426,7 +426,7 @@ public stateid4 downgradeOpen(NFS4Client client, stateid4 stateid, Inode inode,
     public void delegationReturn(NFS4Client client, stateid4 stateid, Inode inode)
             throws ChimeraNFSException {
 
-        Opaque fileId = new Opaque(inode.getFileId());
+        Opaque fileId = inode.getFileIdKey();
         Lock lock = filesLock.get(fileId);
         lock.lock();
         try {
@@ -464,7 +464,7 @@ public void delegationReturn(NFS4Client client, stateid4 stateid, Inode inode)
     public int getShareAccess(NFS4Client client, Inode inode, stateid4 stateid)
             throws ChimeraNFSException {
 
-        Opaque fileId = new Opaque(inode.getFileId());
+        Opaque fileId = inode.getFileIdKey();
         Lock lock = filesLock.get(fileId);
         lock.lock();
         try {
@@ -527,7 +527,7 @@ public int getShareAccess(NFS4Client client, Inode inode, stateid4 stateid)
      */
     void removeOpen(Inode inode, stateid4 stateid) {
 
-        Opaque fileId = new Opaque(inode.getFileId());
+        Opaque fileId = inode.getFileIdKey();
         Lock lock = filesLock.get(fileId);
         lock.lock();
         try {

core/src/main/java/org/dcache/nfs/v4/NFS4Client.java

@@ -628,7 +628,7 @@ public boolean isCallbackNeede() {
     public synchronized StateOwner getOrCreateOwner(byte[] owner, seqid4 seq) throws BadSeqidException {
         StateOwner stateOwner;
         if (_minorVersion == 0) {
-            Opaque k = new Opaque(owner);
+            Opaque k = Opaque.forBytes(owner);
             stateOwner = _owners.get(k);
             if (stateOwner == null) {
                 state_owner4 so = new state_owner4();
@@ -655,7 +655,7 @@ public synchronized StateOwner getOrCreateOwner(byte[] owner, seqid4 seq) throws
      * @param owner client unique state owner
      */
     public synchronized void releaseOwner(byte[] owner) throws StaleClientidException {
-        Opaque k = new Opaque(owner);
+        Opaque k = Opaque.forBytes(owner);
         StateOwner stateOwner = _owners.remove(k);
         if (stateOwner == null) {
             throw new StaleClientidException();

core/src/main/java/org/dcache/nfs/v4/OperationLOCK.java

@@ -118,11 +118,11 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
 
             NlmLock lock = new NlmLock(lockOwner, _args.oplock.locktype, _args.oplock.offset.value,
                     _args.oplock.length.value);
-            context.getLm().lock(inode.getFileId(), lock);
+            context.getLm().lock(inode.getLockKey(), lock);
 
             // ensure, that on close locks will be released
             lock_state.addDisposeListener(s -> {
-                context.getLm().unlockIfExists(inode.getFileId(), lock);
+                context.getLm().unlockIfExists(inode.getLockKey(), lock);
             });
 
             // FIXME: we might run into race condition, thus updating sedid must be fenced!

core/src/main/java/org/dcache/nfs/v4/OperationLOCKT.java

@@ -85,7 +85,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti
 
             NlmLock lock = new NlmLock(lockOwner, _args.oplockt.locktype, _args.oplockt.offset.value,
                     _args.oplockt.length.value);
-            context.getLm().test(inode.getFileId(), lock);
+            context.getLm().test(inode.getLockKey(), lock);
 
             result.oplockt.status = nfsstat.NFS_OK;
 

core/src/main/java/org/dcache/nfs/v4/OperationLOCKU.java

@@ -76,7 +76,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
             NlmLock lock = new NlmLock(lockOwner, _args.oplocku.locktype, _args.oplocku.offset.value,
                     _args.oplocku.length.value);
             try {
-                context.getLm().unlock(inode.getFileId(), lock);
+                context.getLm().unlock(inode.getLockKey(), lock);
             } catch (LockRangeUnavailabeException e) {
                 // posix locks allows unlocking of not locked regions
             }

core/src/main/java/org/dcache/nfs/v4/nlm/AbstractLockManager.java

@@ -26,6 +26,7 @@
 import java.util.concurrent.locks.Lock;
 import java.util.stream.Collectors;
 
+import org.dcache.nfs.util.Opaque;
 import org.dcache.nfs.v4.xdr.nfs4_prot;
 
 /**
@@ -42,49 +43,116 @@ public abstract class AbstractLockManager implements LockManager {
      * @param objId object id.
      * @return exclusive lock.
      */
+    @Deprecated(forRemoval = true)
     abstract protected Lock getObjectLock(byte[] objId);
 
+    /**
+     * Get exclusive lock on objects locks.
+     *
+     * @param objId object id.
+     * @return exclusive lock.
+     */
+    protected Lock getObjectLock(Opaque objId) {
+        return getObjectLock(objId.asBytes());
+    }
+
     /**
      * Get collection of currently used active locks on the object.
      *
      * @param objId object id.
      * @return collection of active locks.
      */
+    @Deprecated(forRemoval = true)
     abstract protected Collection<NlmLock> getActiveLocks(byte[] objId);
 
+    /**
+     * Get collection of currently used active locks on the object.
+     *
+     * @param objId object id.
+     * @return collection of active locks.
+     */
+    protected Collection<NlmLock> getActiveLocks(Opaque objId) {
+        return getActiveLocks(objId.asBytes());
+    }
+
     /**
      * Add {@code lock} to an object.
      *
      * @param objId object id.
      * @param lock lock to add.
      */
+    @Deprecated(forRemoval = true)
     abstract protected void add(byte[] objId, NlmLock lock);
 
+    /**
+     * Add {@code lock} to an object.
+     *
+     * @param objId object id.
+     * @param lock lock to add.
+     */
+    protected void add(Opaque objId, NlmLock lock) {
+        add(objId.asBytes(), lock);
+    }
+
     /**
      * Remove a lock for the given object.
      *
      * @param objId object id.
      * @param lock lock to remove.
      * @return true, if specified lock was removed.
      */
+    @Deprecated(forRemoval = true)
     abstract protected boolean remove(byte[] objId, NlmLock lock);
 
+    /**
+     * Remove a lock for the given object.
+     *
+     * @param objId object id.
+     * @param lock lock to remove.
+     * @return true, if specified lock was removed.
+     */
+    protected boolean remove(Opaque objId, NlmLock lock) {
+        return remove(objId.asBytes(), lock);
+    }
+
     /**
      * Add all locks from a given collection of locks
      *
-     * @param objId
-     * @param locks
+     * @param objId object id.
+     * @param locks locks to add.
      */
+    @Deprecated(forRemoval = true)
     abstract protected void addAll(byte[] objId, Collection<NlmLock> locks);
 
+    /**
+     * Add all locks from a given collection of locks
+     *
+     * @param objId object id.
+     * @param locks locks to add.
+     */
+    protected void addAll(Opaque objId, Collection<NlmLock> locks) {
+        addAll(objId.asBytes(), locks);
+    }
+
     /**
      * Remove all locks specified by {@code locks} associated with the given object.
      *
      * @param objId object id.
      * @param locks collections of locks to remove.
      */
+    @Deprecated(forRemoval = true)
     abstract protected void removeAll(byte[] objId, Collection<NlmLock> locks);
 
+    /**
+     * Remove all locks specified by {@code locks} associated with the given object.
+     *
+     * @param objId object id.
+     * @param locks collections of locks to remove.
+     */
+    protected void removeAll(Opaque objId, Collection<NlmLock> locks) {
+        removeAll(objId.asBytes(), locks);
+    }
+
     @Override
     public void lock(byte[] objId, NlmLock lock) throws LockException {
         Lock dlmLock = getObjectLock(objId);