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 sandboxing to modules #214

Draft
wants to merge 86 commits into
base: master
Choose a base branch
from
Draft

Conversation

hyblocker
Copy link
Contributor

@hyblocker hyblocker commented Jun 10, 2024

This PR will be a major overhaul of how the module system works internally. This does NOT aim to change the API modules interact with in any way - it only aims to be a major improvement in stability and flexibility from the point of view of module developers and end users.

This change will attempt to improve VRCFT's stability and introduce a whole range of flexibility into the program.

Sandboxing shall be implemented in a similar fashion to browser tabs. Each module will be a sub-process, which will communicate to the main VRCFT process using named pipes. Each sub-process will then maintain it's own instance of the UnifiedExpressions class, and share it with the main process over the aforementioned named pipe. This will allow for low-latency IPC between each sub-process and the main process. Solving race conditions will be tricky in a manner which minimises latency, but is a doable task. The main process will then combine the incoming data from each sub-process together into what shall ultimately be sent to applications such as VRChat.

If a sub-process is left running after VRCFT is closed, the sub-process will terminate itself, to eliminate ghost processes. This will either be done using a heartbeat between the main process and each module sub-process, or by having the module sub-processes periodically check for the presence of VRCFT itself in the background, or, likely a combination of both for extra redundancy.

Sandboxing will allow:

  1. Modules which misbehave to not hang VRCFT. For instance, say someone is using a module similar to the Sranipal module, which primarily interacts using a native C++ DLL to the eye and face tracking runtime, and Sranipal crashes for whatever reason. Due to poor programming in the C++ DLL, the DLL would hang VRCFT and force the user to restart VRCFT through task manager. This is not ideal as it is a poor user experience. Sandboxing will allow VRCFT to terminate modules which misbehave such as in the aforementioned example.
  2. Users will now be able to restart, install and uninstall modules without having to restart VRCFT. Since modules will now be sub-processes, restarting, installing and uninstalling will be as simple as launching a sub-process.
  3. Users will also be able to disable and enable parts of the module, so that users can more easily mix different modules with one another. For instance, if one has a Quest Pro and is using a Vive Facial tracker with it, instead of having to fork the Quest Pro module they are using to disable face tracking, they can simply disable it in VRCFT itself.
  4. Developers will be able to develop modules more easily. Since VRCFT will be able to reload modules live, it will enable hot reloading, which shall reduce iteration time for module developers.

This PR is currently a work in progress, but I'm opening a draft pull request so that should any changes be wanted, we may discuss them before I put in the effort in realising them.

This also depends on #213.

@benaclejames
Copy link
Owner

You are an absolute legend! I'll look over this soon but this is exactly what I had been putting off doing since the launch of the modules system

We want to minimise the amount of data transferred between the module and VRCFT. We should use the UE version number to disallow incompatible versions of modules to communicate with the program. We want to minimise the data transferred to minimise latency between modules and destination programs,

I am aiming for a C-like structure with a fixed memory size so that the data is always going to be N bytes large, so that the JIT can optimise it away into a super fast memcpy call.
@hyblocker
Copy link
Contributor Author

Currently, I'm seeing an issue with IPC. Since C# doesn't allow for an easy way of defining fixed size buffers, transferring all blendshapes is going to either be a lot of cumbersome boilerplate code or I will have to use unsafe code so that I can use fixed size buffers. I want to use unsafe since it allows me to copy memory between processes really quickly minimising latency.

The approach I'm taking currently is having a proxy class for Unified Expressions in the sub-process to redirect all writes to a C-like struct, so that then I can cast it to a byte[] and send that over the named pipe very quickly while avoiding marshaling.

I want to ideally end up with a solution similar to what I did in FreeScuba, where data is written to a struct and we copy the struct byte for byte over the named pipe between processes without doing any serialisation (as it's pointless since we're running on the same machine anyway).

@hyblocker
Copy link
Contributor Author

We've opted to use UDP for IPC rather than named pipes as described above. I am currently implementing the IPC protocol and am going to attempt to get a basic form of functional sandboxing implemented soon.

@hyblocker
Copy link
Contributor Author

Managed to implement a full duplex handshake, with support for several sandbox process communicating simultaneously. Getting most of VRCFT modules to work under sandboxing should now mostly be straightforward.

hyblocker added 27 commits July 14, 2024 14:47
Ideally later we can give different timeouts depending on the stage the module is in during it's lifetime
This is to make VRCFT more usable but still convenient for constant debugging and restarts
We should flag language maintainers to update these strings accordingly
MD doc describing how these are formatted coming soon™️
This uses an invalid value of 0xFFFFFFFF currently. Might be worthwhile changing it into a NaN or some similar value later.
@hyblocker
Copy link
Contributor Author

This is nearing the point where the PR will be ready for merging.

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.

2 participants