Skip to content

Conversation

chrfwow
Copy link
Contributor

@chrfwow chrfwow commented Aug 19, 2025

Adds a vmlens check to this repo. This tool will us in the future to find and fix concurrency issues in our code base.
VMLens works by attaching a Java agent during the test run, which will intercept calls inside the allInterleavings.hasNext() loop. The agent goes over all possible iterations of interleavings that threads could do, and thereby discoveres race conditions.

        try (AllInterleavings allInterleavings = new AllInterleavings("Concurrent evaluations")) {
            while (allInterleavings.hasNext()) {
                Runner.runParallel(
                        () -> assertEquals("def", client.getStringValue("a", "a")),
                        () -> assertEquals("as", client.getStringValue("b", "b")));
            }
        }

Asserts inside the tests help to verify that no operation was done on an intermediate state of the application.
Note that iterating over all interleavings can take some time, so the tests should be kept short.

The agent is installed by the vmlens-maven-plugin as needed.

@chrfwow chrfwow changed the title Make hooks thread safer feat: Add vmlens checks Aug 19, 2025
Copy link

codecov bot commented Aug 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.24%. Comparing base (8dd40fa) to head (41b362e).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1567      +/-   ##
============================================
+ Coverage     92.82%   93.24%   +0.42%     
- Complexity      487      489       +2     
============================================
  Files            46       46              
  Lines          1170     1170              
  Branches        103      103              
============================================
+ Hits           1086     1091       +5     
+ Misses           54       49       -5     
  Partials         30       30              
Flag Coverage Δ
unittests 93.24% <100.00%> (+0.42%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chrfwow chrfwow force-pushed the make-hooks-thread-safer branch from 1c5db1d to 14c6f8b Compare August 28, 2025 12:08
Signed-off-by: christian.lutnik <[email protected]>
Signed-off-by: christian.lutnik <[email protected]>
@chrfwow chrfwow marked this pull request as ready for review August 28, 2025 12:37
@chrfwow chrfwow requested review from a team as code owners August 28, 2025 12:37
@toddbaert
Copy link
Member

I definitely think this is cool, and useful.

I have a concern about committing binaries though; to me, these always present a vector for attack in OSS projects, since it's very easy to miss changes to them and nearly impossible to verify their contents.

Could we instead download this files if they don't exist with dependency:get or maybe with maven-download-plugin or even a script (probably not he best option as it will break on windows)?

If you think I'm being overly worried here, please feel free to object. cc @aepfli @beeme1mr @lukas-reining

@chrfwow
Copy link
Contributor Author

chrfwow commented Sep 1, 2025

I agree with you, @toddbaert, having the binary in our code base is not optimal. If we choose to download the jar file during each build run, would you say we have to make sure that we check the integrity of the jar file by comparing it to some hash code, so that we can be sure the file wasn't tampered with?

.gitignore Outdated
@@ -13,3 +13,6 @@ target

# used for spec compliance tooling
java-report.json

# vmlens stuff
/vmlens-agent/vmlens/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we still need this ignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. nice catch

Copy link

sonarqubecloud bot commented Sep 2, 2025

@chrfwow
Copy link
Contributor Author

chrfwow commented Sep 2, 2025

@toddbaert I removed the vmlens .jar file, because they are not needed. The vmlens-maven-plugin installs the agent on its own

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants