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

Notify about Job property change when job is uninstalled #11697

Merged
merged 1 commit into from Feb 12, 2019

Conversation

muktupavels
Copy link
Contributor

Pull request #10963 seems to have introduced regression in systemctl. In Unity and GNOME Flashback logout has stopped working after upgrading systemd to 240. Both sessions are started with:
systemctl --user start --wait target

After some debugging I found that there was 3 PropertiesChanged signals when trying to logout:

  • ActiveState = active, Job = { 338, /org/freedesktop/systemd1/job/338 }
  • ActiveState = active, Job = { 338, /org/freedesktop/systemd1/job/338 }
  • ActiveState = inactive, Job = { 338, /org/freedesktop/systemd1/job/338 }

After that followed also JobRemoved signal for 338, but that did not cause PropertiesChanged signal. Now we have stopped target with systemctl still waiting when that will happen...

Mentioned pull request added unit to dbus queue on job install to notify that Job property has changed. Do same also on job uninstall. With this I am getting 4th PropertiesChanged signal with expected ActiveState = inactive, Job { 0 }.

Commit e6d0591 added unit to dbus
queue on job install. Do same on job uninstall to make sure we get
PropertiesChanged signal.
@yuwata yuwata added the pid1 label Feb 12, 2019
@yuwata yuwata added this to the v241 milestone Feb 12, 2019
@poettering poettering merged commit 52c6c9e into systemd:master Feb 12, 2019
@khurshid-alam
Copy link

This should be picked up by debian for 240. @martinpitt ?

@martinpitt
Copy link
Contributor

@khurshid-alam : I'm currently reviewing the changes between 240 and 241. It's a very bug fix-y release, so if nothing stands out and it passes our testing, I'll instead push 241. Otherwise I'll backport this fix. Thanks for the notification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

5 participants