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

Add support for interrupt #13

Closed

Conversation

sosthene-nitrokey
Copy link
Contributor

Motivation: trussed-dev/trussed#124

This PR contains a bit more than just the added Interrupt method on the channel:

  • It reduces duplication by moving the transition method to the channel
  • It fixes a race condition in the respond implementation:
    Since the check of State::BuildingResponse == self.channel.state.load(Ordering::Acquire) was distinct from the self.channel.state.store(State::Responded, …), the request could be cancelled while the response was written. a3cad6d adds the fix and updates tests/loom.rs to trigger this case.
  • It relaxes the ordering of the transition method.
    The first ordering of no operation relies on SeqCst semantics.
    All works with Acquire/Release semantics, so there is no reason to use the SeqCst ordering.
    The second ordering argument is for the failure, in which case we don't read any data,
    and therefore don't need any synchronization.

@sosthene-nitrokey sosthene-nitrokey marked this pull request as draft May 17, 2023 09:28
Reduce code duplication
The Responder never actually dealt with the intermediary stages of
CancelingRequested and CancelingBuildingResponse, but could still observe them.

Despite that, the loom tests still don't reach any panic
The first ordering of no operation relies on SeqCst semantics.
All works with Acquire/Release semantics, so there is no reason to use the SeqCst ordering.

The second ordering argument is for the failure, in which case we don't read any data,
and therefore don't need any synchronization.
@sosthene-nitrokey sosthene-nitrokey marked this pull request as ready for review May 31, 2023 08:45
@sosthene-nitrokey sosthene-nitrokey marked this pull request as draft May 31, 2023 08:48
@sosthene-nitrokey sosthene-nitrokey mentioned this pull request May 31, 2023
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.

1 participant