On 09/11/17(Thu) 09:02, Helg Bredow wrote:
> The current libfuse signal handling assumes that the file system will always
> be unmounted by the child. One obvious case where this is not true is if the
> file system is busy. To replicate:
>
> 1. mount a fuse file system
> 2. cd anywhere on the file system
> 3. pkill -INT <fuse exe>
>
> The result is a zombie child process and no more response to signals even if
> the file system is no longer busy.
>
> This patch ensures that the child always exits and that an error is printed
> to stdout if the file system cannot be unmounted. Tested with fuse-exfat and
> ntfs_3g. Suggestions for improvement are welcome.
Nice to see that you're fixing a bug. However I'd suggest you to go
much further in your improvement.
Signal handlers are hard and instead of doing work inside the signal
handler you should toggle a global variable/flag and do this work
inside the main loop (fuse_loop()?).
For example your code below calls fprintf(3) in the signal handler. This
is incorrect, this functions is not signal handler safe. What about
fuse_unmount()? Are you sure it can be called from a signal handler?
I'd recommend you to read signal(3) and some signal handlers like the ones
from dhclient(8) and ntpd(8).
> Index: fuse.c
> ===================================================================
> RCS file: /cvs/src/lib/libfuse/fuse.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 fuse.c
> --- fuse.c 4 Nov 2017 13:17:18 -0000 1.34
> +++ fuse.c 9 Nov 2017 08:41:11 -0000
> @@ -303,26 +303,40 @@ ifuse_get_signal(unused int num)
> {
> struct fuse *f;
> pid_t child;
> - int status;
> + int status, err;
> +
> + if (num == SIGCHLD) {
> + /* child fuse_unmount() completed, check error */
> + while (waitpid(WAIT_ANY, &status, WNOHANG) == -1) {
> + if (errno != EINTR)
> + fprintf(stderr, "fuse: %s\n", strerror(errno));
> + }
> +
> + if (WIFEXITED(status)) {
> + err = WEXITSTATUS(status);
> + if (err)
> + fprintf(stderr, "fuse: %s\n", strerror(err));
> + }
> +
> + signal(SIGCHLD, SIG_DFL);
> + return;
> + }
>
> if (sigse != NULL) {
> + signal(SIGCHLD, ifuse_get_signal);
> child = fork();
>
> if (child < 0)
> - return;
> + return ;
>
> - f = sigse->args;
> if (child == 0) {
> + /* try to unmount, file system may be busy */
> + f = sigse->args;
> + errno = 0;
> fuse_unmount(f->fc->dir, f->fc);
> - sigse = NULL;
> - exit(0);
> + _exit(errno);
> }
>
> - fuse_loop(f);
> - while (waitpid(child, &status, 0) == -1) {
> - if (errno != EINTR)
> - break;
> - }
> }
> }
>
>
