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

mount units do not kill background processes when stopped #6048

Closed
neilbrown opened this issue May 29, 2017 · 1 comment
Closed

mount units do not kill background processes when stopped #6048

neilbrown opened this issue May 29, 2017 · 1 comment
Assignees
Labels
bug 🐛 Programming errors, that need preferential fixing pid1
Milestone

Comments

@neilbrown
Copy link
Contributor

Submission type

  • Bug report

systemd version the issue has been seen with

…current upstream

NOTE: Do not submit bug reports about anything but the two most recently released systemd versions upstream!

Used distribution

…openSUSE Factory

In case of bug report: Expected behaviour you didn't see

When a mount unit is stopped, background threads forked by the /sbin/mount.nfs process are not killed.

In case of bug report: Unexpected behaviour you saw

… multiple background threads, one for every time the unit was started.

In case of bug report: Steps to reproduce the problem

  1. create an NFS mount unit for a non-responding server, with "bg" option and short timeount, so it will switch to bg mode before the 90 timeout. e.g.

    192.168.1.111:/nowhere /mnt nfs bg,timeo=10,retrans=1 0 0

  2. start the mount (systemctl start mnt.mount). This will wait for a couple of seconds then appear to succeed. "systemctl status mnt.mount" will show an /sbin/mount.nfs process still running in the background.

  3. stop the mount unit (systemctl stop mnt.mount). The background task is not killed.

  4. repeat "start" and "stop" several times, and find several mount.nfs processes

Note that these processes to respond to SIGTERM. "killall mount.nfs" makes them all go away, and it just uses SIGTERM.

@neilbrown
Copy link
Contributor Author

The problem here is easier to reproduce than above.
Simply have any mount unit where the mount command will take a non-trivial amount of time to complete and run
systemctl --no-block start foo.mount
systemctl stop foo.mount
systemctl status foo.mount

and notice that the mount processes are still running.
systemctl kill foo.mount
will kill them.

@poettering poettering added bug 🐛 Programming errors, that need preferential fixing pid1 labels Jun 6, 2017
@poettering poettering added this to the v235 milestone Jun 6, 2017
@poettering poettering self-assigned this Sep 22, 2017
poettering added a commit to poettering/systemd that referenced this issue Sep 25, 2017
This changes the mount unit state engine in the following ways:

1. The MOUNT_MOUNTING_SIGTERM and MOUNT_MOUNTING_SIGKILL are removed.
   They have been pretty much equivalent to MOUNT_UNMOUNTING_SIGTERM and
   MOUNT_UNMOUNTING_SIGKILL in what they do, and the outcome has been
   the same as well: the unit is stopped. Hence, let's simplify things a
   bit, and merge them. Note that we keep
   MOUNT_REMOUNTING_{SIGTERM|SIGKILL} however, as those states have a
   different outcome: the unit remains started.

2. mount_enter_signal() will now honour the SendSIGKILL= option of the
   mount unit if it was set. This was previously done already when we
   entered the signal states through a timeout, and was simply missing
   here.

3. A new helper function mount_enter_dead_or_mounted() is added that
   places the mount unit in either MOUNT_DEAD or MOUNT_MOUNTED,
   depending on what the kernel thinks about the mount's state. This
   function is called at various places now, wherever we finished an
   operation, and want to make sure our own state reflects again what
   the kernel thinks. Previously we had very similar code in a number of
   places and in other places didn't recheck the kernel state. Let's do
   that with the same logic and function at all relevant places now.

4. Rework mount_stop(): never forget about running control processes.
   Instead: when we have a start (i.e. a /bin/mount) process running,
   and are asked to stop, then enter the kill states for it, so that it
   gets cleaned up. This fixes systemd#6048. Moreover, when we have a reload
   process running convert the possible states into the relevant
   unmounting states, so that we can properly execute the requested
   operation.

Fixes systemd#6048
poettering added a commit to poettering/systemd that referenced this issue Sep 25, 2017
This changes the mount unit state engine in the following ways:

1. The MOUNT_MOUNTING_SIGTERM and MOUNT_MOUNTING_SIGKILL are removed.
   They have been pretty much equivalent to MOUNT_UNMOUNTING_SIGTERM and
   MOUNT_UNMOUNTING_SIGKILL in what they do, and the outcome has been
   the same as well: the unit is stopped. Hence, let's simplify things a
   bit, and merge them. Note that we keep
   MOUNT_REMOUNTING_{SIGTERM|SIGKILL} however, as those states have a
   different outcome: the unit remains started.

2. mount_enter_signal() will now honour the SendSIGKILL= option of the
   mount unit if it was set. This was previously done already when we
   entered the signal states through a timeout, and was simply missing
   here.

3. A new helper function mount_enter_dead_or_mounted() is added that
   places the mount unit in either MOUNT_DEAD or MOUNT_MOUNTED,
   depending on what the kernel thinks about the mount's state. This
   function is called at various places now, wherever we finished an
   operation, and want to make sure our own state reflects again what
   the kernel thinks. Previously we had very similar code in a number of
   places and in other places didn't recheck the kernel state. Let's do
   that with the same logic and function at all relevant places now.

4. Rework mount_stop(): never forget about running control processes.
   Instead: when we have a start (i.e. a /bin/mount) process running,
   and are asked to stop, then enter the kill states for it, so that it
   gets cleaned up. This fixes systemd#6048. Moreover, when we have a reload
   process running convert the possible states into the relevant
   unmounting states, so that we can properly execute the requested
   operation.

Fixes systemd#6048
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Programming errors, that need preferential fixing pid1
Development

No branches or pull requests

2 participants