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

Switch to a multiprocess client-sever model #499

Merged
merged 8 commits into from
May 15, 2021
Merged

Switch to a multiprocess client-sever model #499

merged 8 commits into from
May 15, 2021

Conversation

kunalmohan
Copy link
Member

  • Client and server now run in separate processes. The server process is a daemon.
  • Add a separate panic hook for the server process.
  • Use ClientToServerMsg and ServerToClientMsg for IPC. ServerInstruction is to be used on the server side only and ClientInstruction is used on the client-side only.
  • config_options are sent to the server with NewClient() message only. They are not passed while spawning the server.
  • Tests still run in a single process.

@kunalmohan kunalmohan requested review from khs26 and imsnif May 12, 2021 18:53
@imsnif
Copy link
Member

imsnif commented May 13, 2021

Hey @kunalmohan - good to see this coming along so quickly! I take it detching/attaching is not yet implemented, right? What happens when Zellij is closed? If it's opened more than once, is it connected to the same daemon? If we merge just this change, how will it affect users? Anything else along these lines I'm not thinking about?

@kunalmohan
Copy link
Member Author

@imsnif You are right; sessions detach/attach is not implemented yet. When zellij is closed, the server also exits, so there is no actual need for the server to be a daemon at the moment (I can remove that if that's better, will just have to comment out a couple of lines). Each client has a different server, so if we open 2 instances of zellij, we'll have 4 processes.
The only thing exposed to the users is the --server <socket_path> that spawns a server listening at the socket. The users should not use this manually, as it is used by the client to spawn a server (since client generates the socket file name). There is no additional feature/functionality added or exposed to the user, so they can continue using zellij as before.

The only thing that might be of concern would be- what happens if either the server or the client crashes? I have added the panic hook implementation that ensures (to some extent) that when either of the processes panic, a message is sent to the other one, to exit. This won't work if the panic is in the main thread in either of the processes, which I suspect would rarely happen.

@kunalmohan kunalmohan merged commit 4c198b4 into main May 15, 2021
@kunalmohan kunalmohan deleted the multiprocess branch May 15, 2021 17:33
kunalmohan added a commit that referenced this pull request May 15, 2021
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