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.

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

Removed ST effects from IOSimPOR #3662

Merged
merged 3 commits into from Mar 14, 2022
Merged

Conversation

coot
Copy link
Contributor

@coot coot commented Mar 10, 2022

ST effects are thread local, they never race with other steps. This patch
reverts a previous fix commited in PR #3647.

Thanks for IOSimPOR we also found out that the transition:
UnnegotiatedSt Inbound → TerminatingSt should be valid, similarly to one for
outbound connections (see PR #3652, commit fbcb6aa).

ST computations can not be shared by multiple threads, they are always
thread local; which means they never can race with anything else. This
means we don't need to track ST effects, and consider them when checking
if two steps race or not.
This is a valid transition, e.g. when a handshake fails.  Similar to the
transition `UnnegotiatedSt Outbound → TerminatingSt` which was made
valid in commit fbcb6aa (pr #3632).

Discovered with the help of `IOSimPOR`.
@coot coot added networking io-sim Issues related to io-sim and io-sim-classes. labels Mar 10, 2022
@coot coot requested a review from bolt12 March 10, 2022 17:57
@rjmh
Copy link
Contributor

rjmh commented Mar 11, 2022

This looks good to me.

@coot
Copy link
Contributor Author

coot commented Mar 14, 2022

bors merge

@coot coot added the io-sim-discovered Issue discovered by IOSim label Mar 14, 2022
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 14, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit 62dee5b into master Mar 14, 2022
@iohk-bors iohk-bors bot deleted the coot/io-sim-por-no-st-effects branch March 14, 2022 12:42
coot added a commit that referenced this pull request May 16, 2022
3640: Connection manager transition order test using IOSimPOR r=coot a=coot

This PR adds a quickcheck property test which validates order of connection manager transitions using *IOSimPOR*.  It also fixes a connection manager bug discovered thanks to the new test.

Fixes #3613.

- connection-manager-tests: test transition order
- connection-manager: don't blindly override state
- connection-manager-tests: code style


3662: Removed ST effects from IOSimPOR r=coot a=coot

ST effects are thread local, they never race with other steps.  This patch
reverts a previous fix commited in PR #3647.

Thanks for `IOSimPOR` we also found out that the transition:
`UnnegotiatedSt Inbound → TerminatingSt` should be valid, similarly to one for
outbound connections (see PR #3652, commit fbcb6aa).


Co-authored-by: Marcin Szamotulski <coot@coot.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io-sim Issues related to io-sim and io-sim-classes. io-sim-discovered Issue discovered by IOSim
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants