Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Flaky test or race condition - ParallelSpec#@ResourceLock with only READ allows parallel execution of data-driven features #1600

Open
Vampire opened this issue Mar 13, 2023 · 6 comments · May be fixed by #1610
Open

Flaky test or race condition - ParallelSpec#@ResourceLock with only READ allows parallel execution of data-driven features #1600

Vampire opened this issue Mar 13, 2023 · 6 comments · May be fixed by #1610
Labels
bug

Comments

@Vampire
Copy link
Member

Vampire commented Mar 13, 2023 •
edited
Loading

Describe the bug

I've seen the
ParallelSpec#@ResourceLock with only READ allows parallel execution of data-driven features test failing with

    Condition not satisfied:

    atomicInteger.get() == 3
    |             |     |
    3             1     false
        at apackage.ASpec.writeA(Script_9186d4f70d72d15ebba0478d4c746046.groovy:14)

in master lately in GitHub Action run.

I tried to reproduce by adding @RepeatUntilFailure (nice addition).
First try didn't reproduce in about 1000 runs.
Second try reproced after 4 iterations with the same failure.
Third try didn't reproduce again in about 1000 runs.

So it seems there is some race condition somewhere.

To Reproduce

Add @RepeatUntilFailure to the test and try to make it fail

Expected behavior

No assertion error

Actual behavior

Occasionally assertion error

Java version

Seen with Java 8 and Java 17.
Seen with Groovy 2.5 and Groovy 3.0 variant.

Buildtool version

Gradle as configured in master

What operating system are you using

Windows
@Vampire Vampire added the bug label Mar 13, 2023
@Vampire
Copy link
Member Author

Vampire commented Mar 13, 2023

I wonder how get() can return 1 when toString() returned 3 actually.

@kriegaex
Copy link
Contributor

I wonder how get() can return 1 when toString() returned 3 actually.

Interesting question. Maybe the value has changed in between the evaluations of both? (No, I have not thought this through yet, just guessing in "rubber duck mode".)

@Vampire
Copy link
Member Author

Vampire commented Mar 13, 2023

Yeah, but it "shouldn't" be possible ™️
It is an AtomicInteger that is incremented with incrementAndGet() three times.
And there is a CountDownLatch that waits for the three increments to finish.
And if the CountDownLatch would time out while waiting, there should be an InterruptedException, not this behavior, shouldn't it? :-/

Damn you, multi-thread development. :-D

@Vampire
Copy link
Member Author

Vampire commented Mar 13, 2023

I even quickly added a noExceptionThrown() now to make sure it is not only masked, but I still got the assertion error.

@kriegaex
Copy link
Contributor

Do you think that maybe in rare cases countDownLatch.await(timeout, MILLISECONDS) might time out? Maybe you want to assert on the return value and check if it is false in the problematic case.

spock/spock-specs/src/test/groovy/org/spockframework/smoke/extension/ParallelSpec.groovy

Lines 518 to 524 in 3d7aa63

static int incrementAndBlock(AtomicInteger sharedResource, CountDownLatch countDownLatch, long timeout = 100)
throws InterruptedException {
int value = sharedResource.incrementAndGet()
countDownLatch.countDown()
countDownLatch.await(timeout, MILLISECONDS)
return value
}

Sorry, I did not run or even check out the code, I just had 3 minutes while waiting for a chat reply while working.

@Vampire
Copy link
Member Author

Vampire commented Mar 13, 2023

Oh, you might be right, thanks.
Now that I double-checked I see that await returns false if the timeout occurs.
I thought it will throw an exception in that case, but that's not the case.
That would be good then, as it then is a test-bug, not a code bug. :-)

@Vampire Vampire linked a pull request Mar 26, 2023 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix usages of some async utilities Vampire/spock
2 participants
@Vampire @kriegaex