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

Fix sesman signal processing #2168

Merged
merged 4 commits into from
Mar 15, 2022

Conversation

matt335672
Copy link
Member

@matt335672 matt335672 commented Mar 3, 2022

edit 2022-03-04: No longer draft.

This series of commits fixes #1729 and may also address reports of other session ending problems (e.g. #2021).

The "self-pipe trick" [Stevens 1992] is used to keep the processing performed in all signal handlers to a minimum. This is really simple to do in xrdp by using g_set_wait_obj() and related calls.

As a result, SIGPIPE and SIGHUP are now processed from the sesman main loop, like all other events.

Other problems with SIGHUP processing have been addressed:-

  1. The log file was closed and re-opened. Since the log file is shared with the child processes, this will cause the parent and the children to have a different file table entries. Writes at the end of the file will not be synchronised.
  2. The existing code does not allow for the listening port to be changed, which is a major reason why the user may have edited the config in the first place.

This series of 4 commits addresses these issues as follows:-

  1. [tidy-up] Some of the global variables defined in sesman.c are declared in other c files as extern. This commit moves these declarations into sesman.h
  2. [tidy-up] struct log_config contains a few members which are not always needed. For example, the per_logger_level member is only used for debug builds. This commit makes these members (and associated code) conditional.
  3. The 3rd parameter of the log_start() call is change from a boolean dump_on_start to a more generic flags field. One of the new flags is LOG_START_DUMP_CONFIG, which provides the existing dump_on_start functionality. The other one, LOG_START_RESTART allows for logging to be restarted smoothly. Existing log files are left untouched by a restart, even if the name changes.
  4. Finally, sesman is updated as follows:-
    • Signal handlers now just call g_set_wait_obj(). No significant processing is done in the signal handlers.
    • In the main loop, the wait objects are handled in the same way as file descriptors. All actions resulting from signals are synchronised with actions resulting from file activity.
    • The restart code now calls log_start() with LOG_START_RESTART, and allows the user to change the listening port dynamically.

@matt335672 matt335672 marked this pull request as ready for review March 4, 2022 11:57
@matt335672
Copy link
Member Author

As another note, when developing with sesman, it's now really easy to change the logging level, or add entries to [LoggingPerLogger]. After editing the config, a sudo pkill -HUP xrdp-sesman causes the file to be re-read without needing to stop and start sesman.

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.

session end code is not signal-safe
2 participants