fix: call proceedNextContext() in reset() and replace test sleeps with semaphores

Fixes #988

LockableResourcesManager.reset() cleared resource state but never called
proceedNextContext(), so waiting pipeline jobs were never notified when
resources became free via reset(). Every other method that frees resources
(unlockResources, unreserve, addResource) already does this. Added the
same while(proceedNextContext()){} loop inside reset().

Replaced all sleep() calls in the reserve/setReservedBy-inside-lock tests
with SemaphoreStep-based synchronisation, making the tests deterministic
and roughly 4x faster (~20s vs ~80s each). Removed the try-catch Bug#2a
and Bug#2b tolerance blocks since the reset() fix makes them unnecessary.

Affected tests:
- LockStepReserveInsideLockHonouredTest
- LockStepSetReservedByInsideLockHonouredTest
- LockStepTest#reserveInsideLockHonoured
- LockStepTest#setReservedByInsideLockHonoured

Author: mPokornyETM

src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java

@@ -1174,6 +1174,11 @@ public void reset(List<LockableResource> resources) {
                 uncacheIfFreeing(r, true, true);
                 r.reset();
             }
+
+            while (proceedNextContext()) {
+                // process as many contexts as possible
+            }
+
             save();
         }
         scheduleQueueMaintenance();

src/test/java/org/jenkins/plugins/lockableresources/LockStepReserveInsideLockHonouredTest.java

@@ -4,6 +4,7 @@
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
+import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
 import org.junit.jupiter.api.Test;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
@@ -39,7 +40,6 @@ void reserveInsideLockHonoured(JenkinsRule j) throws Exception {
                         + "    def res = "
                         + lmget
                         + ".reserve([lr], 'test2a')\n"
-                        // + "    semaphore 'wait-inside'\n"
                         + "    echo \"Locked resource cause 1-2a: ${lr.getLockCause()}\"\n"
                         + "    echo \"Locked resource reservedBy 1-2a: ${lr.getReservedBy()}\"\n"
                         + "    if (!res) {\n"
@@ -49,34 +49,27 @@ void reserveInsideLockHonoured(JenkinsRule j) throws Exception {
                         + "        echo \"Locked resource reservedBy 1-2b: ${lr.getReservedBy()}\"\n"
                         + "    }\n"
                         + "    echo \"Unlocking parallel closure 1\"\n"
+                        + "    semaphore 'p1-inside'\n" // Java controls when p1 exits lock
                         + "  }\n"
                         + "  echo \"Locked resource cause 1-3 (after unlock): ${lr.getLockCause()}\"\n"
                         + "  echo \"Locked resource reservedBy 1-3: ${lr.getReservedBy()}\"\n"
-                        + "  echo \"Ended locked parallel closure 1 with resource reserved, sleeping...\"\n"
-                        + "  sleep (5)\n"
+                        + "  echo \"Ended locked parallel closure 1 with resource reserved\"\n"
                         + "  echo \"Locked resource cause 1-4: ${lr.getLockCause()}\"\n"
                         + "  echo \"Locked resource reservedBy 1-4: ${lr.getReservedBy()}\"\n"
-                        + "  echo \"Resetting Locked resource via LRM and sleeping ...\"\n"
+                        + "  semaphore 'p1-before-reset'\n" // Java asserts p2 hasn't started before reset
+                        + "  echo \"Resetting Locked resource via LRM\"\n"
                         + "  "
                         + lmget
                         + ".reset([lr])\n"
-                        + "  sleep (5)\n"
-                        + "  echo \"Un-reserving Locked resource via LRM and sleeping...\"\n"
+                        + "  echo \"Un-reserving Locked resource via LRM\"\n"
                         + "  "
                         + lmget
                         + ".unreserve([lr])\n"
-                        + "  sleep (5)\n"
-                        // Note: the unlock attempt here might steal this resource
-                        // from another parallel stage, so we don't do it:
-                        // + "  echo \"Un-locking Locked resource via LRM and sleeping...\"\n"
-                        // + "  " + lmget + ".unlock([lr], null)\n"
-                        // + "  sleep (5)\n"
+                        + "  semaphore 'p1-after-reset'\n" // Java ensures p2 started before p1 logs 1-5
                         + "  echo \"Locked resource cause 1-5: ${lr.getLockCause()}\"\n"
                         + "  echo \"Locked resource reservedBy 1-5: ${lr.getReservedBy()}\"\n"
-                        + "  sleep (5)\n"
                         + "  if (lr.getLockCause() == null) {\n"
                         + "    echo \"LRM seems stuck; trying to reserve/unreserve this resource by LRM methods\"\n"
-                        // + "    lock(label: 'label1') { echo \"Secondary lock trick\" }\n"
                         + "    if ("
                         + lmget
                         + ".reserve([lr], 'unstucker')) {\n"
@@ -86,76 +79,71 @@ void reserveInsideLockHonoured(JenkinsRule j) throws Exception {
                         + ".unreserve([lr])\n"
                         + "    } else { echo \"Could not reserve by LRM methods as 'unstucker'\" }\n"
                         + "  }\n"
-                        + "  sleep (5)\n"
                         + "  echo \"Locked resource cause 1-6: ${lr.getLockCause()}\"\n"
                         + "  echo \"Locked resource reservedBy 1-6: ${lr.getReservedBy()}\"\n"
                         + "},\n"
                         + "p2: {\n"
-                        // + "  semaphore 'wait-outside'\n"
                         + "  org.jenkins.plugins.lockableresources.LockableResource lr = null\n"
                         + "  echo \"Locked resource cause 2-1: not locked yet\"\n"
                         + "  lock(label: 'label1', variable: 'someVar2') {\n"
                         + "    echo \"VAR2 IS $env.someVar2\"\n"
                         + "    lr = "
                         + lmget
                         + ".fromName(env.someVar2)\n"
-                        + "    sleep (1)\n"
                         + "    echo \"Locked resource cause 2-2: ${lr.getLockCause()}\"\n"
                         + "    echo \"Locked resource reservedBy 2-2: ${lr.getReservedBy()}\"\n"
                         + "    echo \"Setting (directly) and dropping (via LRM) a reservation on locked resource\"\n"
                         + "    lr.reserve('test2-1')\n"
-                        + "    sleep (3)\n"
                         + "    "
                         + lmget
                         + ".unreserve([lr])\n"
-                        + "    echo \"Just sleeping...\"\n"
-                        + "    sleep (20)\n"
+                        + "    semaphore 'p2-holding'\n" // Java controls when p2 sets test2-2 and exits
                         + "    echo \"Setting (directly) a reservation on locked resource\"\n"
                         + "    lr.reserve('test2-2')\n"
                         + "    echo \"Unlocking parallel closure 2\"\n"
                         + "  }\n"
                         + "  echo \"Locked resource cause 2-3: ${lr.getLockCause()}\"\n"
                         + "  echo \"Locked resource reservedBy 2-3: ${lr.getReservedBy()}\"\n"
-                        + "  sleep (5)\n"
                         + "  echo \"Recycling (via LRM) the reserved not-locked resource\"\n"
+                        + "  semaphore 'p2-before-recycle'\n" // Java asserts p3 hasn't got the lock before recycle
                         + "  "
                         + lmget
                         + ".recycle([lr])\n"
-                        + "  sleep (5)\n"
+                        + "  semaphore 'p2-after-recycle'\n" // Java ensures p3 started before p2 logs 2-4
                         + "  echo \"Locked resource cause 2-4: ${lr.getLockCause()}\"\n"
                         + "  echo \"Locked resource reservedBy 2-4: ${lr.getReservedBy()}\"\n"
                         + "},\n"
                         // Test that reserve/unreserve in p2 did not "allow" p3 to kidnap the lock:
                         + "p3: {\n"
                         + "  org.jenkins.plugins.lockableresources.LockableResource lr = null\n"
                         + "  echo \"Locked resource cause 3-1: not locked yet\"\n"
-                        + "  sleep 1\n"
+                        + "  semaphore 'p3-wait'\n" // Java controls when p3 enters the queue
                         + "  lock(label: 'label1', variable: 'someVar3') {\n"
                         + "    echo \"VAR3 IS $env.someVar3\"\n"
                         + "    lr = "
                         + lmget
                         + ".fromName(env.someVar3)\n"
                         + "    echo \"Locked resource cause 3-2: ${lr.getLockCause()}\"\n"
                         + "    echo \"Locked resource reservedBy 3-2: ${lr.getReservedBy()}\"\n"
-                        + "    echo \"Just sleeping...\"\n"
-                        + "    sleep (10)\n"
                         + "    echo \"Unlocking parallel closure 3\"\n"
+                        + "    semaphore 'p3-in-lock'\n" // Java controls when p3 exits lock
                         + "  }\n"
                         + "  echo \"Locked resource cause 3-3: ${lr.getLockCause()}\"\n"
                         + "  echo \"Locked resource reservedBy 3-3: ${lr.getReservedBy()}\"\n"
                         + "},\n"
                         // Add some pressure to try for race conditions:
-                        + "p4: { sleep 2; lock(label: 'label1') { sleep 1 } },\n"
-                        + "p5: { sleep 2; lock(label: 'label1') { sleep 3 } },\n"
-                        + "p6: { sleep 2; lock(label: 'label1') { sleep 2 } },\n"
-                        + "p7: { sleep 2; lock(label: 'label1') { sleep 1 } },\n"
-                        + "p8: { sleep 2; lock(label: 'label1') { sleep 2 } },\n"
-                        + "p9: { sleep 2; lock(label: 'label1') { sleep 1 } }\n"
+                        + "p4: { semaphore 'p4'; lock(label: 'label1') { } },\n"
+                        + "p5: { semaphore 'p5'; lock(label: 'label1') { } },\n"
+                        + "p6: { semaphore 'p6'; lock(label: 'label1') { } },\n"
+                        + "p7: { semaphore 'p7'; lock(label: 'label1') { } },\n"
+                        + "p8: { semaphore 'p8'; lock(label: 'label1') { } },\n"
+                        + "p9: { semaphore 'p9'; lock(label: 'label1') { } }\n"
                         + "\necho \"Survived the test\"\n"
                         + "}", // timeout wrapper
                 false));
         WorkflowRun b1 = p.scheduleBuild2(0).waitForStart();
 
+        // --- Phase 1: p1 holds lock exclusively (p2,p3 blocked) ---
         j.waitForMessage("Locked resource cause 1-1", b1);
         j.assertLogNotContains("Locked resource cause 2-2", b1);
         j.assertLogNotContains("Locked resource cause 2-3", b1);
@@ -164,99 +152,86 @@ void reserveInsideLockHonoured(JenkinsRule j) throws Exception {
         j.assertLogNotContains("Locked resource cause 2-2", b1);
         j.assertLogNotContains("Locked resource cause 2-3", b1);
 
+        // Let p1 exit lock closure
+        SemaphoreStep.success("p1-inside/1", null);
+
+        // --- Phase 2: p1 released lock but resource is still reserved ---
         j.waitForMessage("Locked resource cause 1-3", b1);
         j.assertLogNotContains("Locked resource cause 2-2", b1);
         j.assertLogNotContains("Locked resource cause 2-3", b1);
 
-        // Bug #1 happens here (without further patch):
-        // although resource is seen as reserved, it is
-        // grabbed anyway by the other parallel thread
-        // which is already waiting. Notably, log is like:
-        //  62.908 [setReservedByInsideLockHonoured #1] Lock acquired on [Label: label1]
-        //  62.909 [setReservedByInsideLockHonoured #1] Lock released on resource [Label: label1]
-        // and the consistent ordering of acquired first,
-        // released later is unsettling.
+        // Bug #1: although resource is reserved, p2 must NOT grab it.
         j.waitForMessage("Locked resource cause 1-4", b1);
-        // Note: stage in test has a sleep(1) to reduce chances that
-        // this line is noticed in log although it is there AFTER 1-4:
         j.assertLogNotContains("Locked resource cause 2-2", b1);
         j.assertLogNotContains("Locked resource cause 2-3", b1);
         LOGGER.info("GOOD: Did not encounter Bug #1 " + "(parallel p2 gets the lock on a still-reserved resource)!");
 
+        // --- Phase 3: p1 calls reset() → p2 gets lock via proceedNextContext ---
+        // Ensure p2 is in the waiting queue before p1 calls reset()
+        j.waitForMessage("[Label: label1] is not free, waiting for execution ...", b1);
+
+        // p1 calls reset+unreserve, then blocks at semaphore p1-after-reset.
+        // p2 gets the lock (dispatched by proceedNextContext), enters closure,
+        // does reserve/unreserve cycle, then blocks at semaphore p2-holding.
+        SemaphoreStep.success("p1-before-reset/1", null);
+        SemaphoreStep.waitForStart("p2-holding/1", b1);
+
+        // Release p3 and pressure stages into the lock queue while p2 holds the lock
+        SemaphoreStep.success("p3-wait/1", null);
+        for (String s : new String[] {"p4", "p5", "p6", "p7", "p8", "p9"}) {
+            SemaphoreStep.success(s + "/1", null);
+        }
+
+        // Now let p1 continue to log 1-5 (p2 already holds the lock)
+        SemaphoreStep.success("p1-after-reset/1", null);
+
         j.waitForMessage("Locked resource cause 1-5", b1);
-        // This line might not strictly be required,
-        // but we are processing a parallel pipeline
-        // and many seconds were spent sleeping, so:
         j.assertLogContains("Locked resource cause 2-1", b1);
-        // Here the other parallel stage may have already started
-        // (we try to recycle the resource between 1-4 and 1-5):
-        // j.assertLogNotContains("Locked resource cause 2-2", b1);
-        // j.assertLogNotContains("Locked resource cause 2-3", b1);
+        j.assertLogContains("Locked resource cause 2-2", b1);
+        LOGGER.info("GOOD: Parallel 2 started after Parallel 1 reset the resource!");
 
-        // Bug #2 happens here: even after the resource is known
-        // to be un-reserved, resources already looping waiting
-        // for it (after the fix for Bug #1) are not "notified".
-        // Adding and removing the resource helps unblock this.
-        boolean sawBug2a = false;
-        try {
-            j.waitForMessage("Locked resource cause 1-6", b1);
-            j.assertLogContains("Locked resource cause 2-2", b1);
-        } catch (java.lang.AssertionError t1) {
-            sawBug2a = true;
-            LOGGER.info("Bug #2a (Parallel 2 did not start after Parallel 1 finished "
-                    + "and resource later released) currently tolerated");
-            // LOGGER.info(t1.toString());
-            // throw t1;
-        }
-        if (!sawBug2a) {
-            LOGGER.info("GOOD: Did not encounter Bug #2a "
-                    + "(Parallel 2 did not start after Parallel 1 finished "
-                    + "and resource later released)!");
-        }
+        // p2 holds the lock, reservation cycle (test2-1 → unreserve) already done
+        j.assertLogContains("Locked resource reservedBy 1-5: null", b1);
 
-        // If the bug is resolved, then by the time we get to 1-5
-        // the resource should be taken by the other parallel stage
-        // and so not locked by not-"null"; reservation should be away though
-        // Note: Due to parallel execution timing, p2 may have already called
-        // reserve('test2-1') before p1 logs point 1-5, so we tolerate both states
-        boolean sawBug2b = false;
-        try {
-            j.assertLogContains("Locked resource reservedBy 1-5: null", b1);
-        } catch (java.lang.AssertionError t) {
-            // p2 set a reservation before p1 logged 1-5 - this is expected timing variation
-            j.assertLogContains("Locked resource reservedBy 1-5: test2-1", b1);
-            LOGGER.info("Timing variation: p2 reserved resource before p1 logged 1-5 state");
-        }
+        // The resource is locked by p2 at point 1-5, so "cause 1-5: null" and the
+        // un-stucking workaround should never appear.
         for (String line : new String[] {
             "Locked resource cause 1-5: null", "LRM seems stuck; trying to reserve/unreserve", "Secondary lock trick"
         }) {
-            try {
-                j.assertLogNotContains(line, b1);
-            } catch (java.lang.AssertionError t2) {
-                sawBug2b = true;
-                LOGGER.info("Bug #2b (LRM required un-stucking) currently tolerated: " + line);
-                // LOGGER.info(t2.toString());
-                // throw t2;
-            }
-        }
-        if (!sawBug2b) {
-            LOGGER.info("GOOD: Did not encounter Bug #2b " + "(LRM required un-stucking)!");
+            j.assertLogNotContains(line, b1);
         }
+        LOGGER.info("GOOD: Resource was properly locked at point 1-5 (no un-stucking needed)!");
+
+        j.waitForMessage("Locked resource cause 1-6", b1);
+        j.assertLogContains("Locked resource cause 2-2", b1);
 
         j.waitForMessage("Locked resource cause 2-2", b1);
         j.assertLogContains("Locked resource cause 1-5", b1);
-        LOGGER.info("GOOD: lock#2 was taken after we un-reserved lock#1");
+        LOGGER.info("GOOD: lock#2 was taken after p1 reset/released the resource");
 
-        j.waitForMessage("Unlocking parallel closure 2", b1);
+        // --- Phase 4: p2 exits lock (still reserved as test2-2) → p3 must NOT get in ---
+        SemaphoreStep.success("p2-holding/1", null);
+
+        // Wait for p2 to reach the semaphore before recycle — lock is released but reserved
+        SemaphoreStep.waitForStart("p2-before-recycle/1", b1);
+        j.assertLogContains("Unlocking parallel closure 2", b1);
         j.assertLogNotContains("Locked resource cause 3-2", b1);
         LOGGER.info("GOOD: lock#3 was NOT taken just after we un-locked closure 2 (keeping lock#2 reserved)");
 
-        // After 2-3 we lrm.recycle() the lock so it should
-        // go to the next bidder
+        // --- Phase 5: p2 recycles → p3 gets lock ---
+        // p2 calls recycle(), p3 gets the lock, then p2 blocks at p2-after-recycle.
+        // Wait for p3 to actually log 3-2 before letting p2 log 2-4.
+        SemaphoreStep.success("p2-before-recycle/1", null);
+        j.waitForMessage("Locked resource cause 3-2", b1);
+        SemaphoreStep.success("p2-after-recycle/1", null);
+
         j.waitForMessage("Locked resource cause 2-4", b1);
         j.assertLogContains("Locked resource cause 3-2", b1);
         LOGGER.info("GOOD: lock#3 was taken just after we recycled lock#2");
 
+        // Let p3 finish
+        SemaphoreStep.success("p3-in-lock/1", null);
+
         j.assertLogContains(", waiting for execution ...", b1);
 
         j.assertBuildStatusSuccess(j.waitForCompletion(b1));