Declaring the global read-write lock on a leaf node should not fail execution

[fslev]

I've upgraded my Cucumber test framework with JUnit 5.11.1 and I get a new error:

[ERROR]   Task was deferred but should have been executed synchronously: NodeTestTask [PickleDescriptor: [engine:junit-platform-suite]/[suite:com.ionos.cloud.cucumber.ml.MlApiTest]/[engine:cucumber]/[feature:classpath%3Afeatures%2FModel%2FModelLB.feature]/[scenario:26]]

All I can say is that inside the feature file I have two scenarios: one marked with @isolated global read write exclusive resource and another scenario is marked with a simple read write exclusive resource.

Reverting to JUnit 5.11.0 works without any error !

I'm sorry I cannot give more details. Maybe I will try to isolate it to a more detailed scenario when I'll have more time.

[mpkorstanje]

This can be reproduced with https://github.com/mpkorstanje/junit5-locks/tree/main

Feature: example

  @isolated
  Scenario: a
    When I wait 1 hour

  @reads-and-writes-system-properties
  Scenario: b
    When I wait 1 hour

cucumber.execution.parallel.enabled=true
cucumber.execution.parallel.config.strategy=fixed
cucumber.execution.parallel.config.fixed.parallelism=2
cucumber.execution.exclusive-resources.isolated.read-write=org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_KEY
cucumber.execution.exclusive-resources.reads-and-writes-system-properties.read-write=java.lang.System.properties

package io.cucumber.skeleton;

import io.cucumber.java.en.When;

import java.util.concurrent.TimeUnit;

public class StepDefinitions {
    @When("I wait {int} hour")
    public void iWaitHour(int arg0) throws InterruptedException {
        TimeUnit.SECONDS.sleep(arg0);
    }
}

Unlike with JUnit Jupiter the ExclusiveResource.GLOBAL_KEY is not used at top level:

example
 - a  <- locks ExclusiveResource.GLOBAL_KEY
 - b <-  locks java.lang.System.properties

Prior to executing b, the nop lock from example and the exclusive resource from a is still held in the thread locks.

resourceLock = {SingleLock@3016} "SingleLock [resource = ExclusiveResource [key = 'java.lang.System.properties', lockMode = READ_WRITE]]"
 resources = {Collections$SingletonList@3025}  size = 1
  0 = {ExclusiveResource@3029} "ExclusiveResource [key = 'java.lang.System.properties', lockMode = READ_WRITE]"
 lock = {ReentrantReadWriteLock$WriteLock@3026} "java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock@2c79993b[Unlocked]"
threadLock = {ForkJoinPoolHierarchicalTestExecutorService$ThreadLock@2471} 
 locks = {ArrayDeque@3023}  size = 2
  0 = {SingleLock@2677} "SingleLock [resource = ExclusiveResource [key = 'org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_KEY', lockMode = READ_WRITE]]"
  1 = {NopLock@2217} "NopLock []"

So then ResourceLock.isCompatible returns false because

junit5/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ResourceLock.java

Lines 81 to 85 in d013085

	boolean isGlobalReadLock = ownResources.size() == 1
	&& ExclusiveResource.GLOBAL_READ.equals(ownResources.get(0));
	if ((!isGlobalReadLock && other.isExclusive()) || this.isExclusive()) {
	return false;
	}

And it is worth noting that the comment about the ExclusiveResource.GLOBAL_KEY only applies to JUnit Jupiter.

junit5/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ResourceLock.java

Lines 78 to 80 in d013085

	// The global read lock (which is always on direct children of the engine node)
	// needs special treatment so that it is compatible with the first write lock
	// (which may be on a test method).

[marcphilipp]

Does Cucumber somehow bypass NodeTreeWalker which ensures the global lock is acquired at the top level?

[mpkorstanje]

Cucumber does not. I'd say that the CucumberTestEngine is fairly unremarkable in that regard.

The difference is that JUnits @Isolated can only be applied at class level. And classes are always the children of a test engine. Where as in Cucumber the the global lock is applied at what would be JUnits method level. At a glance this seems to run counter to assumptions with which the NodeTreeWalker was written. I'd have to debug it a bit more to be certain but I haven't got a long enough stretch of time available for that right now.

Note: It doesn't look like any actual parallelism is needed to reproduce the problem. This will also produce the problem:

cucumber.execution.parallel.config.fixed.parallelism=1
cucumber.execution.parallel.config.fixed.max-pool-size=1

[mpkorstanje]

With some trickery it is possible effectively put @Isolated on a method. This results in the error message as with Cucumber.

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.ResourceLock;

import java.util.concurrent.TimeUnit;

import static org.junit.jupiter.api.parallel.ResourceAccessMode.READ_WRITE;
import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_KEY;

public class LockTest {

  @Test
  @ResourceLock(value = GLOBAL_KEY, mode = READ_WRITE) // effectively @Isolated
  void test1() throws InterruptedException {
    TimeUnit.SECONDS.sleep(1);
  }

  @Test
  @ResourceLock(value = "b", mode = READ_WRITE)
  void test2() throws InterruptedException {
    TimeUnit.SECONDS.sleep(1);
  }

}

junit.jupiter.execution.parallel.mode.default=concurrent
junit.jupiter.execution.parallel.enabled=true
junit.jupiter.execution.parallel.config.strategy=fixed
junit.jupiter.execution.parallel.config.fixed.parallelism=1

[marcphilipp]

Thanks for the reproducer! I'll take a look today.

[marcphilipp]

I've verified that #4039 fixes the problem for the reproducer.

[mpkorstanje]

Cheers. That was quick!

[marcphilipp]

Backported to the releases/5.11.x branch ✔️