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

Integrate async-init pattern to network/Connection.ts #293

Closed
CMCDragonkai opened this issue Nov 23, 2021 · 4 comments · Fixed by #292
Closed

Integrate async-init pattern to network/Connection.ts #293

CMCDragonkai opened this issue Nov 23, 2021 · 4 comments · Fixed by #292
Assignees
Labels
development Standard development r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices

Comments

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Nov 23, 2021

Specification

The network/Connection.ts and derived classes are still using the old manual start booleans. These should have async-init applied as well. I recommend using CreateDestroy instead of start stop.

Make sure to update ForwardProxy and ReverseProxy as well as the entire connection object lifecycle is managed by these 2.

Because Connection is an abstract class, the pattern has to follow the same style as GRPCClient. The static creation function instead creates parameters used to construct the derived classes.

Additional context

Tasks

  1. Update the Connection, ConnectionForward, ConnectionReverse and replace start/stop with CreateDestroy pattern
  2. Update ForwardProxy and ReverseProxy with the new create destroy patterns used by Connection.
  3. Ensure that network tests are working
  4. Ensure that nodes tests are working
@CMCDragonkai CMCDragonkai added the development Standard development label Nov 23, 2021
@CMCDragonkai
Copy link
Member Author

It'd be important to revert some of the changes done to networking until a proper review @tegefaulkes

@CMCDragonkai
Copy link
Member Author

The ForwardProxy and ReverseProxy is already StartStop pattern this is because their start relies on parameters that only exist when GRPC related objects have started. But also the GRPC related objects can only start once they have properties that PolykeyAgent has when it's already constructed. This means due to transitivity, both of these do not have asynchronous creation.

The ConnectionForward and ConnectionReverse can be made into CreateDestroy since both uses of them are always started as soon as they are created. However due to the usage of abstract class, then the pattern is applied to the ConnectionForward and ConnectionReverse directly.

@CMCDragonkai
Copy link
Member Author

After attempting to do this, asynchronous creation is going to be difficult for ConnectionForward and ConnectionReverse as they have conditions like resolveReadyP which is assigned to the instance and used by handlers like handleMessage.

If seems making it CreateDestroy would requires bigger overhaul then it deserves right now. So instead I will just keep it as StartStop as it works this way.

@CMCDragonkai
Copy link
Member Author

So it's now StartStop. Note that composed is a separate state from being running or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices
Development

Successfully merging a pull request may close this issue.

2 participants