-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Support and document SIGHUP for xrdp-sesman #2416
Conversation
Isn't it a little bit asymmetric to do the kill with "xrdp-sesman --kill" and the reload with "kill -HUP $MAINPID". I think it would be cleaner to either remove the "--kill" code from sesman or add a "--reload" option to sesman. But that's probably just me being my usual obsessive-compulsive self... Anyway, I think your commit will be doing the job just fine. Thanks for the fix and the documentation. |
Hi @yxzzx Thanks for the swift reply. No - that's a very good point. It absolutely should be consistent. Leave it with me. |
64cfec9
to
c99f609
Compare
Adds a --reload switch to sesman and plumbs this in to systemctl reload xrdp-sesman.service
c99f609
to
3a0a932
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM.
BTW, is the log file reopened after reloading?
The code for a log restart was added for 2484928 :- Lines 727 to 782 in 4ff968b
Since the log file is shared, we never change it, as we've got no way of telling any child processes that the log file has changed. If the user tries to change it we log a warning and carry on using the old one. That's the current behaviour anyway. |
Will this also affect #1317 ? |
@danielperna84 - thanks for the heads-up. As far as I can tell, #1317 isn't actually a problem. More information would be required to figure out exactly what was going on there. |
Fixes #2414
@yxzzx - can you take a look at this please?