Pesomka/perf test

pom.xml

@@ -73,8 +73,8 @@
             <groupId>org.slf4j</groupId>
             <artifactId>slf4j-log4j12</artifactId>
             <version>1.7.7</version>
-            <scope>test</scope>
         </dependency>
+
     </dependencies>
 
     <build>

src/main/java/com/microsoft/aad/msal4jextensions/CrossProcessCacheFileLock.java

@@ -5,35 +5,34 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.io.RandomAccessFile;
 import java.lang.management.ManagementFactory;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Cross process lock based on OS level file lock.
+ * Cross process lock based on lock file creation/deletion (optional) + OS level file lock.
  */
 class CrossProcessCacheFileLock {
-
     private final static Logger LOG = LoggerFactory.getLogger(CrossProcessCacheFileLock.class);
-    public static final String READ_MODE = "r";
-    public static final String WRITE_MODE = "rw";
 
     private int retryDelayMilliseconds;
     private int retryNumber;
 
     private File lockFile;
-    private RandomAccessFile randomAccessFile;
-
-    private String mode;
 
     private FileLock lock;
 
+    private FileChannel fileChannel;
+
+    private boolean locked;
+
     /**
      * Constructor
      *
@@ -43,113 +42,134 @@ class CrossProcessCacheFileLock {
      */
     CrossProcessCacheFileLock(String lockfileName, int retryDelayMilliseconds, int retryNumber) {
         lockFile = new File(lockfileName);
-        lockFile.deleteOnExit();
 
         this.retryDelayMilliseconds = retryDelayMilliseconds;
         this.retryNumber = retryNumber;
     }
 
-    /**
-     * Acquire read lock - can be shared by multiple readers
-     *
-     * @throws CacheFileLockAcquisitionException
-     */
-    void readLock() throws CacheFileLockAcquisitionException {
-        lock(READ_MODE);
-    }
-
-    /**
-     * Acquire write lock - exclusive access
-     *
-     * @throws CacheFileLockAcquisitionException
-     */
-    void writeLock() throws CacheFileLockAcquisitionException {
-        lock(WRITE_MODE);
-    }
-
-    private String getProcessId(){
+    private String getProcessId() {
         String vmName = ManagementFactory.getRuntimeMXBean().getName();
 
-        String pid = vmName.substring(0, vmName.indexOf("@"));
-
-        return pid;
+        return vmName.substring(0, vmName.indexOf("@"));
     }
 
     private String getLockProcessThreadId() {
         return "pid:" + getProcessId() + " thread:" + Thread.currentThread().getId();
     }
 
+    private boolean tryToCreateLockFile() {
+        for (int tryCount = 0; tryCount < retryNumber; tryCount++) {
+            boolean fileCreated = false;
+            try {
+                fileCreated = lockFile.createNewFile();
+            } catch (IOException ex) {
+                LOG.error(ex.getMessage());
+            }
+            if (fileCreated) {
+                return true;
+            } else {
+                waitBeforeRetry();
+            }
+        }
+        return false;
+    }
+
+    private void waitBeforeRetry(){
+        try {
+            Thread.sleep(retryDelayMilliseconds);
+        } catch (InterruptedException e) {
+            LOG.error(e.getMessage());
+        }
+    }
+
     /**
-     * Tries to acquire OS lock for lock file
+     * Tries to acquire lock by creating lockFile (optional),
+     * and acquiring OS lock for lockFile (mandatory)
      * Retries {@link #retryNumber} times with {@link #retryDelayMilliseconds} delay
      *
      * @throws CacheFileLockAcquisitionException if the lock was not obtained.
      */
-    private void lock(String mode) throws CacheFileLockAcquisitionException {
+    void lock() throws CacheFileLockAcquisitionException {
+        if (!tryToCreateLockFile()) {
+            LOG.debug(getLockProcessThreadId() + " Failed to create lock file!");
+        }
+
         for (int tryCount = 0; tryCount < retryNumber; tryCount++) {
             try {
                 lockFile.createNewFile();
 
-                LOG.debug(getLockProcessThreadId() + " acquiring " + mode + " file lock");
-
-                randomAccessFile = new RandomAccessFile(lockFile, mode);
-                FileChannel channel = randomAccessFile.getChannel();
+                LOG.debug(getLockProcessThreadId() + " acquiring file lock");
+                fileChannel = FileChannel.open(lockFile.toPath(),
+                        StandardOpenOption.READ,
+                        StandardOpenOption.SYNC,
+                        StandardOpenOption.WRITE);
 
-                boolean isShared = READ_MODE.equals(mode);
-                lock = channel.lock(0L, Long.MAX_VALUE, isShared);
-
-                this.mode = mode;
+                // try to get file lock
+                lock = fileChannel.tryLock();
+                if (lock == null) {
+                    throw new IllegalStateException("Lock is not available");
+                }
 
-                if (!lock.isShared()) {
-                    String jvmName = java.lang.management.ManagementFactory.getRuntimeMXBean().getName();
+                // for debugging purpose write jvm name to lock file
+                writeJvmName(fileChannel);
 
-                    // for debugging purpose
-                    // if exclusive lock acquired, write jvm name to lock file
-                    ByteBuffer buff = ByteBuffer.wrap(jvmName.replace("@", " ").
-                            getBytes(StandardCharsets.UTF_8));
-                    channel.write(buff);
-                }
-                LOG.debug(getLockProcessThreadId() + " acquired file lock, isShared - " + lock.isShared());
+                locked = true;
+                LOG.debug(getLockProcessThreadId() + " acquired OK file lock");
                 return;
             } catch (Exception ex) {
-                LOG.debug(getLockProcessThreadId() + " failed to acquire " + mode + " lock," +
+                LOG.debug(getLockProcessThreadId() + " failed to acquire lock," +
                         " exception msg - " + ex.getMessage());
-                try {
-                    releaseResources();
-                } catch (IOException e) {
-                    LOG.error(e.getMessage());
-                }
-
-                try {
-                    Thread.sleep(retryDelayMilliseconds);
-                } catch (InterruptedException e) {
-                    LOG.error(e.getMessage());
-                }
+                releaseResources();
+                waitBeforeRetry();
             }
         }
-        LOG.error(getLockProcessThreadId() + " failed to acquire " + mode + " lock");
+        LOG.error(getLockProcessThreadId() + " failed to acquire lock");
 
         throw new CacheFileLockAcquisitionException(
-                getLockProcessThreadId() + " failed to acquire " + mode + " lock");
+                getLockProcessThreadId() + " failed to acquire lock");
+    }
+
+    private void writeJvmName(FileChannel fileChannel) throws IOException {
+        String jvmName = ManagementFactory.getRuntimeMXBean().getName();
+
+        ByteBuffer buff = ByteBuffer.wrap(jvmName.replace("@", " ").
+                getBytes(StandardCharsets.UTF_8));
+        fileChannel.write(buff);
     }
 
     /**
-     * Release OS lock for lockFile
+     * Release OS lock for lockFile,
+     * delete lockFile if it was created by lock() method
      *
      * @throws IOException
      */
     void unlock() throws IOException {
-        LOG.debug(getLockProcessThreadId() + " releasing " + mode + " lock");
+        LOG.debug(getLockProcessThreadId() + " releasing lock");
 
         releaseResources();
+
+        if (locked) {
+            deleteLockFile();
+        }
     }
 
-    private void releaseResources() throws IOException {
-        if (lock != null) {
-            lock.release();
+    private void deleteLockFile() throws IOException {
+        if (!Files.deleteIfExists(lockFile.toPath())) {
+            LOG.debug(getLockProcessThreadId() + " FAILED to delete lock file");
+        }
+    }
+
+    private void releaseResources() {
+        try {
+            if (lock != null) {
+                lock.release();
+            }
+            if (fileChannel != null) {
+                fileChannel.close();
+            }
         }
-        if (randomAccessFile != null) {
-            randomAccessFile.close();
+        catch (IOException ex){
+            LOG.error(ex.getMessage());
         }
     }
 }

src/main/java/com/microsoft/aad/msal4jextensions/PersistenceSettings.java

@@ -312,7 +312,12 @@ public Builder setLockRetry(int delayMilliseconds, int retryNumber) {
          * @return An immutable instance of {@link com.microsoft.aad.msal4jextensions.PersistenceSettings}.
          */
         public PersistenceSettings build() {
-            PersistenceSettings persistenceSettings = new PersistenceSettings(
+            if (keyringSchemaName != null && linuxUseUnprotectedFileAsCacheStorage) {
+                throw new IllegalArgumentException(
+                        "Only one type of persistence can be used on Linux - KeyRing or Unprotected file");
+            }
+
+            return new PersistenceSettings(
                     cacheFileName,
                     cacheDirectoryPath,
                     keychainService,
@@ -327,8 +332,6 @@ public PersistenceSettings build() {
                     linuxUseUnprotectedFileAsCacheStorage,
                     lockRetryDelayMilliseconds,
                     lockRetryNumber);
-
-            return persistenceSettings;
         }
     }
 }

src/main/java/com/microsoft/aad/msal4jextensions/PersistenceTokenCacheAccessAspect.java

@@ -35,7 +35,7 @@ public class PersistenceTokenCacheAccessAspect implements ITokenCacheAccessAspec
     private PersistenceSettings parameters;
 
     private String getCacheLockFilePath() {
-        return parameters.getCacheDirectoryPath() + File.separator + ".lock";
+        return parameters.getCacheDirectoryPath() + File.separator + ".lockfile";
     }
 
     private String getCacheFilePath() {
@@ -94,7 +94,7 @@ private boolean isReadAccess(ITokenCacheAccessContext iTokenCacheAccessContext)
         return !isWriteAccess(iTokenCacheAccessContext);
     }
 
-    private void updateLastSeenCacheFileModifiedTimestamp() throws IOException {
+    private void updateLastSeenCacheFileModifiedTimestamp() {
         lastSeenCacheFileModifiedTimestamp = getCurrentCacheFileModifiedTimestamp();
     }
 
@@ -106,14 +106,14 @@ public Long getCurrentCacheFileModifiedTimestamp() {
     public void beforeCacheAccess(ITokenCacheAccessContext iTokenCacheAccessContext) {
         try {
             if (isWriteAccess(iTokenCacheAccessContext)) {
-                lock.writeLock();
+                lock.lock();
             } else {
                 Long currentCacheFileModifiedTimestamp = getCurrentCacheFileModifiedTimestamp();
                 if (currentCacheFileModifiedTimestamp != null &&
                         currentCacheFileModifiedTimestamp.equals(lastSeenCacheFileModifiedTimestamp)) {
                     return;
                 } else {
-                    lock.readLock();
+                    lock.lock();
                 }
             }
             byte[] data = cacheAccessor.read();
@@ -138,8 +138,6 @@ public void afterCacheAccess(ITokenCacheAccessContext iTokenCacheAccessContext)
                 cacheAccessor.write(iTokenCacheAccessContext.tokenCache().serialize().getBytes(StandardCharset.UTF_8));
                 updateLastSeenCacheFileModifiedTimestamp();
             }
-        } catch (IOException ex) {
-            LOG.error(ex.getMessage());
         } finally {
             try {
                 lock.unlock();