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

Fixes: When systemd tries to kill nodm, nodm restarts itself instead #5 #10

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

slashblog
Copy link

No description provided.

Fix for reboot problem
Fixes When systemd tries to kill nodm, nodm restarts itself instead spanezz#5
Fixes: When systemd tries to kill nodm, nodm restarts itself instead spanezz#5
@slashblog
Copy link
Author

The issue is that shutdown_quit_notification blocks signals. In nodm_display_manager_wait functions there is waitpid call. This returns -1 if signal was received before session (dm->session.pid) was closed by systemctl. This is the happy scenario.

However, if session gets closed before signal was sent to the process. The waitpid returns dm->session.pid and the call to shutdown_quit_notification blocks SIGTERM. Due to this, catch_signals is never called and nodm proceeds to restarting display manager.

I have removed function shutdown_quit_notification altogether, as I don't see any need for it, since IMO there is no need to block these signals.

@sunweaver
Copy link
Collaborator

@slashblog: Thanks for working on issue #5.

However, your pull request is formalistically unclean. I'd be happy, if you are willing to...

To achieve this, you probably need to use git on the command line or some Git GUI. The Github WebUI stuff will be insufficient for handling this, I guess.

Are you willing to perform the requested changes? If not, please let me know. For an updated branch, please do a git push --force :. This will automatically update this PR.

THANKS!

@avindra
Copy link

avindra commented Sep 27, 2017

@sunweaver Github allows you to:

  • Auto squash commits upon merging
  • Change the commit message before merging.

without having the original author (@slashblog) getting involved.

@Lauszus
Copy link

Lauszus commented Oct 23, 2017

Just tested this on a Raspberry Pi running Raspbian Jessie and this PR solves the issue with nodm restarting itself when it is shutting down.

@j00ch
Copy link

j00ch commented Nov 11, 2017

I tested this commit on my Manjaro system, running Openbox and systemd. No fix here sadly, still get the PID error and then nodm restarts itself.

@darkspace
Copy link

This fixed the restart issue on my Intel Compute Stick running Ubuntu 16.04. Thanks @slashblog!

@dustydecapod
Copy link

For the love of god, someone merge this already.

@spanezz
Copy link
Owner

spanezz commented Feb 21, 2018

I am not active in nodm maintenance anymore. @sunweaver, can you look into this, or else add other people as maintainers?

@sunweaver
Copy link
Collaborator

sunweaver commented Feb 21, 2018 via email

@kaey
Copy link

kaey commented Mar 27, 2018

Fix works for me

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.

None yet

9 participants