forkoff()

[Markus Friedl]

On Mon, Feb 04, 2002 at 03:00:19PM -0500, Nicolas Williams wrote:
> Please review the function below, forkoff(), meant to be used in
> clientloop.c instead of daemon() and the code in process_escapes().
> 
> The intention is to make ~D ( like ~& but also detach) possible and to
> make it possible for ssh -f (or ssh -f -f - see other thread on this)
> to detach, not just forkoff().
> 
> I also intend to use the same detach technique in a feature patch for
> the hang-on-exit behaviour that's been hashed to death. I.e., I might
> like to add an option to control ssh's behaviour when the remote session
> exits, the behaviours being: wait for the channel to be closed [current,
> default], close all channels and exit, background, detach and force
> exit.
> 
> To make detaching and some of the hang-on-exit options work correctly I
> need a way to force a channel closed and to do it in way that is
> correct wrt to the SSHv2 protocol. The forkoff() function below does
> that, I think, but it would be nice to have confirmation.
> 
> Comments?
> 
> Nico
> 
> static
> void
> forkoff(int stop_listening, int detach)
> {
> 	pid_t pid;
> 	int fd;
> 	Channel *c;
> 
> 	/* Restore tty modes. */
> 	leave_raw_mode();
> 
> 	/* Stop listening for new connections. */
> 	if (stop_listening)
> 		channel_stop_listening();
> 
> 	fprintf(stderr, "Forking off into the background - %s",
> 		stop_listening ? "no longer listening" : "still listening");
> 
> 	/* Fork into background. */
> 	pid = fork();
> 	if (pid < 0) {
> 		error("fork: %.100s", strerror(errno));
> 		return;
> 	}
> 	if (pid != 0) {	/* This is the parent. */
> 		/* The parent just exits. */
> 		exit(0);
> 	}
> 
> 	c = channel_lookup(session_ident);

^^ there is no channel for ssh1
in ssh1 login sessions are different from, say, tcp-fwded channels.

> 	if (c == NULL)
> 		error("couldn't lookup session channel");
> 
> 	/* The child continues serving connections. */
> 	/* fake EOF on stdin */
> 	if (compat20) {
> 		buffer_append(&stdin_buffer, "\004", 1);

i don't like the \004 magic in the current code.

> 	} else if (!stdin_eof) {
> 		/*
> 		 * Sending SSH_CMSG_EOF alone does not always appear
> 		 * to be enough.  So we try to send an EOF character
> 		 * first.
> 		 */
> 		packet_start(SSH_CMSG_STDIN_DATA);
> 		packet_put_string("\004", 1);
> 		packet_send();
> 		/* Close stdin. */
> 		stdin_eof = 1;
> 		if (buffer_len(&stdin_buffer) == 0) {
> 			packet_start(SSH_CMSG_EOF);
> 			packet_send();
> 		}
> 	}
> 
> 	if (detach) {
> 		/*
> 		 * There should be a chan_wont_read()/chan_wont_write()
> 		 * API, differing only in the debug messages used.
> 		 */
> 		chan_read_failed(c);
> 		chan_write_failed(c);
^^^ you cannot call chan_xxx_failed for every channel state,
	please check nchan2.ms

> 		channel_close_fds(c);

you still want to keep the channel? why?

> 		fd = open(_PATH_DEVNULL, O_RDWR, 0);
> 		if (fd < 0)
> 			return;
> 		(void) dup2(fd, STDIN_FILENO);
> 		(void) dup2(fd, STDOUT_FILENO);
> 		(void) dup2(fd, STDERR_FILENO);
> 		if (fd > 2)
> 			(void) close(fd);
> 		(void) setsid();
> 	}

i'm not sure this works like expected.