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 a SSCSM controller and environment skeleton #15568

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

Conversation

Desour
Copy link
Contributor

@Desour Desour commented Dec 17, 2024

A small step towards SSCSM.

Overview

SSCSMs run in a separate SSCSMEnvironment thread, so we get the same control flow as later with a sandboxed process.
The client interacts with it via the SSCSMController.

Terminology:

  • Request: A sort of API call from the SSCSM thread to the main thread.
  • Answer: The return value of this call.
  • Event: Something offered from the main thread to the SSCSM thread. (Has no return value, see also note below.)

Details

From the SSCSM thread's perspective, it always processes events from the main thread. To get the next event, it does a request to pull the next event. => It's a pull-style API, events are just a special case of answer.

If the main thread wants to trigger an event, it "returns" the event ("return", because the SSCSM thread is currently in a state where it waits for the next event), and then handles requests until the next event is requested (which marks the end of this event's handling).

Note: Events are not meant to be triggered from arbitrary places. When you trigger an event, you have to handle all the requests, which may be arbitrary, because they come from untrusted code. So if you have local state (e.g. a pointer to a node meta), it will be invalidated. (Triggering an event from within a request handling code also doesn't work at all, because the SSCSM is not in the state where it waits for the next event.)
Hence, triggering an even from deep within a function is inherently unsafe. It should be safe to trigger events from step() and from client packet handlers. (If you need to, I guess you can delay an event trigger to the next step, i.e. as an asynchronous event.)
This restriction is an important part of SSCSM security, because if we only trigger events from such places, handling SSCSM API calls is pretty much as secure as handling network packages (assuming a working sandbox).

Quick guide

To add a new event (e.g. on_chat_message):

  • Add a struct SSCSMEventOnChatMessage in sscsm_events.h.
  • PCall the lua function in this struct's exec() member function.
  • In the future: Add de-/serialization functions for this struct.
  • Add a SSCSMController::eventOnChatMessage() member function. (Could maybe remove this part, and instead call runEvent() directly.)
  • Call this function.

To add a new request (e.g. core.get_timeofday()):

  • Add a struct SSCSMRequestGetTimeOfDay in sscsm_requests.h, as well as a SSCSMAnswerGetTimeOfDay.
  • In the struct's exec() member function, get do the stuff you want to do with this request, and return the result in the answer.
  • In the future: Add de-/serialization functions for these structs.
  • Add a SSCSMEnvironment::getTimeOfDay() member function.
  • Add a l_get_time_of_day, where you can now call this function.

What comes next?

As next steps I suggest (concurrently):

  • Replace the dynamic cast stuff with serialization (see turkeymcmac's PR). (And after that, use the IPC channel from my PR. (And then replace the thread with a process. (And then do the process sandbox.)))
  • Load client-provided builtin code.
    (E.g. add an event for adding a file to a virtual filesystem (hashtable virtual path to file data), and an event to then load mods.)

To do

This PR is a Ready for Review.

How to test

  • Start a game and exit.

@tigercoding56
Copy link

tigercoding56 commented Dec 18, 2024

finally progress however slow.

class SSCSMEnvironment;
class StupidChannel;

class SSCSMController
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we name this something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have something in mind?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really. here's a few I can come up with:

  • RemoteController
  • IPCController
  • just CSMController since "CPCSM" was never intended anyway
  • ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarification, the purpose of this class is to:

  • Be the RAII owner of the SSCSM process.
  • Send events to SSCSM process, and process requests. (runEvent)
  • Hide details (e.g. that it is a separate process, or that it has to do IPC calls).

So I think SSCSMController is a suitable name.

RemoteController

The SSCSM process is not remote.

IPCController

It's not about IPC, but about the SSCSM stuff. The IPC thing is internal.

just CSMController since "CPCSM" was never intended anyway

CPCSM currently takes the name CSM / client scripting everywhere (we should rename it imo). So this would in practice be a name clash, and cause confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants