Skip to content

Fix race condition in async_driver#181

Merged
knoepfel merged 1 commit intoFramework-R-D:mainfrom
knoepfel:fix-race-condition
Dec 11, 2025
Merged

Fix race condition in async_driver#181
knoepfel merged 1 commit intoFramework-R-D:mainfrom
knoepfel:fix-race-condition

Conversation

@knoepfel
Copy link
Member

@knoepfel knoepfel commented Dec 11, 2025

It can happen that the function executed by the async_driver throws an exception before the driver updates its state from "off" to "drive". When this happens, the state of the driver goes from "off" to "park" (due to the exception), and then to "drive". This is an error, and it causes a race condition. Instead, the states should go from "off" to "drive" (on the thread that invokes the driver function) to "park" (which can happen either due to normal completion or an exception).

This PR implements this fix.

@codecov
Copy link

codecov bot commented Dec 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #181      +/-   ##
==========================================
- Coverage   80.93%   80.80%   -0.14%     
==========================================
  Files         115      114       -1     
  Lines        2046     2037       -9     
  Branches      330      328       -2     
==========================================
- Hits         1656     1646      -10     
- Misses        256      257       +1     
  Partials      134      134              
Flag Coverage Δ
unittests 80.80% <100.00%> (-0.14%) ⬇️

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

Files with missing lines Coverage Δ
phlex/utilities/async_driver.hpp 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a56582b...88c3b5a. Read the comment docs.

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

@greenc-FNAL
Copy link
Contributor

Is async_driver() accessible from more than one thread (i.e. access to gear_ should be guarded)? It might be possible to define the type of the state enum as std::<atomic> rather than going full guard, but I suspect the call to driver() would need to be grouped under the same guard as gear_

@knoepfel
Copy link
Member Author

Is async_driver() accessible from more than one thread (i.e. access to gear_ should be guarded)? It might be possible to define the type of the state enum as std::<atomic> rather than going full guard, but I suspect the call to driver() would need to be grouped under the same guard as gear_

It turns out that the states enumeration cannot be represented as an std::atomic<int> (https://godbolt.org/z/cbzq3xMj7).
However, gear_ is of type std::atomic<states>. Also, there is only one async_driver per framework program, and access to it is serialized.

@knoepfel knoepfel merged commit 7640bdf into Framework-R-D:main Dec 11, 2025
36 checks passed
@knoepfel knoepfel deleted the fix-race-condition branch December 11, 2025 16:03
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.

4 participants