fix some concurrency issues

Author: roberttoyonaga

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrThrottler.java

@@ -78,7 +78,7 @@ public JfrThrottler() {
      */
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     private void normalize(long samplesPerPeriod, double periodMs) {
-        assert rwlock.isWriteOwner();
+        assert rwlock.isCurrentThreadWriteOwner();
         // Do we want more than 10samples/s ? If so convert to samples/s
         double periodsPerSecond = 1000.0 / periodMs;
         double samplesPerSecond = samplesPerPeriod * periodsPerSecond;
@@ -157,20 +157,20 @@ public boolean sample() {
      */
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     private void rotateWindow() {
-        assert rwlock.isWriteOwner();
+        assert rwlock.isCurrentThreadWriteOwner();
         configure();
         installNextWindow();
     }
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     private void installNextWindow() {
-        assert rwlock.isWriteOwner();
+        assert rwlock.isCurrentThreadWriteOwner();
         activeWindow = getNextWindow();
     }
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     private JfrThrottlerWindow getNextWindow() {
-        assert rwlock.isWriteOwner();
+        assert rwlock.isCurrentThreadWriteOwner();
         if (window0 == activeWindow) {
             return window1;
         }
@@ -179,7 +179,7 @@ private JfrThrottlerWindow getNextWindow() {
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     private long computeAccumulatedDebtCarryLimit(long windowDurationNs) {
-        assert rwlock.isWriteOwner();
+        assert rwlock.isCurrentThreadWriteOwner();
         if (periodNs == 0 || windowDurationNs >= TimeUtils.nanosPerSecond) {
             return 1;
         }
@@ -188,7 +188,7 @@ private long computeAccumulatedDebtCarryLimit(long windowDurationNs) {
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     private long amortizeDebt(JfrThrottlerWindow lastWindow) {
-        assert rwlock.isWriteOwner();
+        assert rwlock.isCurrentThreadWriteOwner();
         if (accumulatedDebtCarryCount == accumulatedDebtCarryLimit) {
             accumulatedDebtCarryCount = 1;
             return 0; // reset because new settings have been applied
@@ -203,7 +203,7 @@ private long amortizeDebt(JfrThrottlerWindow lastWindow) {
      */
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     private void setSamplePointsAndWindowDuration() {
-        assert rwlock.isWriteOwner();
+        assert rwlock.isCurrentThreadWriteOwner();
         assert reconfigure;
         JfrThrottlerWindow next = getNextWindow();
         long samplesPerWindow = eventSampleSize / WINDOW_DIVISOR;
@@ -224,7 +224,7 @@ private void setSamplePointsAndWindowDuration() {
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     private void configure() {
-        assert rwlock.isWriteOwner();
+        assert rwlock.isCurrentThreadWriteOwner();
         JfrThrottlerWindow next = getNextWindow();
 
         // Store updated parameters to both windows.
@@ -257,7 +257,7 @@ protected static double windowLookback(JfrThrottlerWindow window) {
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     private double projectPopulationSize(long lastWindowMeasuredPop) {
-        assert rwlock.isWriteOwner();
+        assert rwlock.isCurrentThreadWriteOwner();
         avgPopulationSize = exponentiallyWeightedMovingAverage(lastWindowMeasuredPop, ewmaPopulationSizeAlpha, avgPopulationSize);
         return avgPopulationSize;
     }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/utils/JfrReadWriteLock.java

@@ -37,7 +37,8 @@ public class JfrReadWriteLock {
     private static final long CURRENTLY_WRITING = Long.MAX_VALUE;
     private final UninterruptibleUtils.AtomicLong ownerCount;
     private final UninterruptibleUtils.AtomicLong waitingWriters;
-    private volatile long writeOwnerTid;
+    private volatile long writeOwnerTid; // If this is set, then a writer owns the lock. Otherwise
+                                         // -1.
 
     public JfrReadWriteLock() {
         ownerCount = new UninterruptibleUtils.AtomicLong(0);
@@ -65,39 +66,42 @@ public void readTryLock(int retries) {
         int yields = 0;
         for (int i = 0; i < retries; i++) {
             long readers = ownerCount.get();
-            // Only attempt to enter the critical section if no writers are waiting or writes
-            // in-progress.
+            // Only begin the attempt to enter the critical section if no writers are waiting or
+            // writes are in-progress.
             if (waitingWriters.get() > 0 || readers == CURRENTLY_WRITING) {
                 yields = maybeYield(i, yields);
             } else {
-                // Attempt to take the lock
+                // Attempt to take the lock.
                 if (ownerCount.compareAndSet(readers, readers + 1)) {
                     return;
                 }
             }
         }
     }
 
-    @Uninterruptible(reason = "This method does not do a transition, so the whole critical section must be uninterruptible.", callerMustBe = true)
+    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     public void writeTryLock(int retries) {
-        int yields = 0;
-        // Increment the writer to count to signal intent.
+        // Increment the writer count to signal our intent to acquire the lock.
         waitingWriters.incrementAndGet();
-        for (int i = 0; i < retries; i++) {
-            long readers = ownerCount.get();
-            // Only enter the critical section if all in-progress readers have finished.
-            if (readers != 0) {
-                yields = maybeYield(i, yields);
-            } else {
-                // Attempt to take the lock
-                if (ownerCount.compareAndSet(0, CURRENTLY_WRITING)) {
-                    // Success. Signal no longer waiting.
-                    long waiters = waitingWriters.decrementAndGet();
-                    assert waiters >= 0;
-                    writeOwnerTid = SubstrateJVM.getCurrentThreadId();
-                    return;
+        try {
+            int yields = 0;
+            for (int i = 0; i < retries; i++) {
+                long readers = ownerCount.get();
+                // Only enter the critical section if all in-progress readers have finished.
+                if (readers != 0) {
+                    yields = maybeYield(i, yields);
+                } else {
+                    // Attempt to acquire the lock.
+                    if (ownerCount.compareAndSet(0, CURRENTLY_WRITING)) {
+                        writeOwnerTid = SubstrateJVM.getCurrentThreadId();
+                        return;
+                    }
                 }
             }
+        } finally {
+            // Regardless of whether we eventually acquired the lock, signal we are done waiting.
+            long waiters = waitingWriters.decrementAndGet();
+            assert waiters >= 0;
         }
     }
 
@@ -122,12 +126,20 @@ private static int maybeYield(int retryCount, int yields) {
 
     @Uninterruptible(reason = "Used in locking without transition, so the whole critical section must be uninterruptible.", callerMustBe = true)
     public void unlock() {
+        if (writeOwnerTid < 0) {
+            // Readers own the lock.
+            long readerCount = ownerCount.decrementAndGet();
+            assert readerCount >= 0;
+            return;
+        }
+        // A writer owns the lock.
+        assert isCurrentThreadWriteOwner();
         writeOwnerTid = -1;
         ownerCount.set(0);
     }
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    public boolean isWriteOwner() {
+    public boolean isCurrentThreadWriteOwner() {
         return writeOwnerTid == SubstrateJVM.getCurrentThreadId();
     }
 }