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

Create an asynchronous server loop #34

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

h7x4
Copy link
Collaborator

@h7x4 h7x4 commented Oct 26, 2023

Asynchronous server loop

Note

Marking this as draft for now, due to severe test failures

If anyone comes by and sees this PR, and wants the functionality it provides, feel free to pick up development.
I don't think I'll be able to work on this for quite a while.


This depends on #31

This PR creates an alternative server loop, in which the incoming URB requests are handled in parallel. There are some kinds of devices which rely on this feature, where it might send several requests to different interfaces, or even just different endpoints on the same interface before the device will send the URB responses.

The flow would look something like this:

sequenceDiagram
    HOST->>IN EP: BULK IN REQ
    HOST->>OUT EP: BULK OUT REQ
    OUT EP->>HOST: BULK OUT RES
    IN EP->>HOST: BULK IN RES
Loading

In order to solve this, I've made use of the async-scoped library, to allow us to keep API with the socket trait object from UsbIpServer::handler. If I've understood it correctly, this is because the trait object is not static and lives on the stack, while normal multithreaded async tokio requires everything to have a static lifetime, as it cannot provide any guarantees about how long the threads are going to live. The library solves this by providing unsafe async functions that assumes you've ensured the threads will be shut down before closing the scope.

This library also seem to be part of the reason why the tests are failing. I am not exactly sure what is going on, but the MockSocket tests are flaky and the TCP Socket tests seem to crash or deadlock. I believe there is either one or more race conditions, which leads to the wakers potentially getting dropped. However, this only seems to be the test driver, as the implementation has been working with some basic interaction testing.

The current design shares the socket between two threads, where one thread (the rx thread) is responsible for handling the incoming packages, and transferring responses to the other thread (the tx thread). The tx thread is responsible for sending all incoming messages. In the event that an USBIP_CMD_SUBMIT is requested, the rx thread will spin up a new thread for handling this request and then sending the response to the tx thread. This ensures the rx thread is ready to receive new requests, and can then spin up multiple USBIP_CMD_SUBMIT handler threads simultaneously.

Breaking Changes

  • Now depends on async-trait and async-scoped
  • UsbIpServer has been remade into a trait
  • The old UsbIpServer has been renamed to SyncUsbIpServer and is now namespaced beneath usbip::server
  • There's a new AsyncUsbIpServer struct which implements UsbIpServer, also located at usbip::server
  • Most of the functions in the usbip namespace (in earlier lib.rs) has been moved into the UsbIpServer trait
  • Added Send trait restriction to handler socket.
  • Added Clone restriction to UsbIpServer, meaning the Arc is no longer required when working with it.

Some remaining TODOs of varying importance:

  • Fix rest of the implementation and tests
  • Switch from async-trait to upcoming rust async traits
  • Write some more tests, especially ones verifying that the code can handle requests in parallel.
  • Clean up some .unwrap()s
  • Deduplicate some of the code, especially the tests.
  • Add some more rigid and human readable documentation than the API docs,
    explaining the difference between the implementations, and when to use which one.
  • Make the traces, warnings, etc. more consistent accross the implementations.
  • Handle graceful shutdown of USBIP_CMD_SUBMIT handler threads in a cleaner way (there's nothing aborting them when rx/tx thread dies, they'll currently just die off on their own whenever the device is finished doing whatever it needs to)
  • See if the final implementation will allow for dropping some trait requirements for the socket in UsbIpServer::handler

@h7x4 h7x4 added the enhancement New feature or request label Oct 26, 2023
@h7x4 h7x4 force-pushed the contribute-async-server-loop branch from fac8068 to b898bd3 Compare October 26, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant