xrdp icon indicating copy to clipboard operation
xrdp copied to clipboard

[WIP] use g_fork_execvp instead of g_fork

Open unstabler opened this issue 1 year ago • 1 comments

DESCRIPTION

  • This PR moved from https://github.com/team-unstablers/xrdp-tumod/pull/1
  • On macOS, some System APIs (including VideoToolbox) which communicates with Mach/XPC services stops working after fork().
    • This PR fixes problem that initialization of VTCompressionSession fails after fork().
  • However, I think there is only one of the above items that will be improved, even though quite a bit of core code has been modified, so you may feel free to close this PR if you don't think it will be necessary. machine-translated

CHANGES

  • g_fork_execvp(exec, argv) has added
  • g_get_executable_path(exectype, buf, bufsize) has added
  • xrdp now accepts --child-process and --child-fd [fd], entrypoint for child process also added
  • xrdp will use g_fork_execvp() instead of g_fork() for accepting new connections

TODO

  • [ ] rollback 4d1b20a : should i reconstruct logger by passing logger->fd to child process (via argv, or ...), as i reconstructed struct trans?
  • [x] remove unreachable / unused code: xrdp_listen.c:814 ...
  • [ ] clean-up struct xrdp_process *, struct trans * after xrdp_process_main_loop

translated from: ですが、かなりコアなコードが修正されたのに、改善される事項は上記の一つしかいないと思うので、もし必要にならないと思われた場合は気楽にこのPRをクローズしても結構です。

unstabler avatar Mar 18 '23 10:03 unstabler

Hi @unstabler

This is going along the right lines, and for the reasons you state regarding MacOS, we need to get this functionality in one way or another.

There's a couple of things you haven't considered which will need to be included in a complete solution:-

  • The sub-process will need to re-read the config file and re-open the log file.
  • The config file may be overridden with the -c switch passed to xrdp.
  • There's some stuff around setting FD_CLOEXEC on existing file descriptors which isn't in the main product yet, but will be added soon.

I'm currently focussed on getting sesman split into two for xrdp v1.x. This will mean I'm going to need to cover all of the above in sesman anyway, so I suggest we hold off on this one until I've got that done. I've probably missed a few things, and we'll find those during testing.

Regarding your existing code, you can simplify things by ditching the --child-process switch and simply using --child-fd. Unless I'm missing something, I can't see a use-case for having one without the other.

matt335672 avatar Mar 21 '23 08:03 matt335672