- markus@cvs.openbsd.org 2001/10/09 21:59:41

     [channels.c channels.h serverloop.c session.c session.h]
     simplify session close: no more delayed session_close, no more blocking wait() calls.

diff --git a/ChangeLog b/ChangeLog
index 2eda5e1b349f9f370dca42d6ebd00dfb38ed23a9..9fd4abbb1c07d786b7b0f2d9086205ac5b6250fc 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -44,6 +44,10 @@
      [serverloop.c]
      close all channels if the connection to the remote host has been closed,
      should fix sshd's hanging with WCHAN==wait
+   - markus@cvs.openbsd.org 2001/10/09 21:59:41
+     [channels.c channels.h serverloop.c session.c session.h]
+     simplify session close: no more delayed session_close, no more 
+     blocking wait() calls.
 
 20011007
  - (bal) ssh-copy-id corrected permissions for .ssh/ and authorized_keys.
@@ -6689,4 +6693,4 @@
  - Wrote replacements for strlcpy and mkdtemp
  - Released 1.0pre1
 
-$Id: ChangeLog,v 1.1597 2001/10/10 05:08:36 djm Exp $
+$Id: ChangeLog,v 1.1598 2001/10/10 05:14:37 djm Exp $

diff --git a/channels.c b/channels.c
index aaa0ea57924fb06adc9904ce7ea5ce5c73d3f55f..04efd7287f7ea5e6aaa82f5711a673893d573cfe 100644 (file)
--- a/channels.c
+++ b/channels.c
@@ -39,7 +39,7 @@
  */
 
 #include "includes.h"
-RCSID("$OpenBSD: channels.c,v 1.138 2001/10/08 11:48:57 markus Exp $");
+RCSID("$OpenBSD: channels.c,v 1.139 2001/10/09 21:59:41 markus Exp $");
 
 #include "ssh.h"
 #include "ssh1.h"
@@ -359,22 +359,6 @@ channel_free_all(void)
                        channel_free(channels[i]);
 }
 
-void
-channel_detach_all(void)
-{
-       int i;
-       Channel *c;
-
-       for (i = 0; i < channels_alloc; i++) {
-               c = channels[i];
-               if (c != NULL && c->detach_user != NULL) {
-                       debug("channel_detach_all: channel %d", c->self);
-                       c->detach_user(c->self, NULL);
-                       c->detach_user = NULL;
-               }
-       }
-}
-
 /*
  * Closes the sockets/fds of all channels.  This is used to close extra file
  * descriptors after a fork.

diff --git a/channels.h b/channels.h
index 49a9df9dd7dd37f4d8731e4bd6a02c19632156cd..090d2ca6e64601cdfa9927ab6d600c8b77d25ee8 100644 (file)
--- a/channels.h
+++ b/channels.h
@@ -32,7 +32,7 @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
-/* RCSID("$OpenBSD: channels.h,v 1.48 2001/10/07 17:49:40 markus Exp $"); */
+/* RCSID("$OpenBSD: channels.h,v 1.49 2001/10/09 21:59:41 markus Exp $"); */
 
 #ifndef CHANNEL_H
 #define CHANNEL_H
@@ -143,7 +143,6 @@ Channel *channel_new(char *, int, int, int, int, int, int, int, char *, int);
 void    channel_set_fds(int, int, int, int, int, int);
 void    channel_free(Channel *);
 void    channel_free_all(void);
-void    channel_detach_all(void);
 void    channel_stop_listening(void);
 
 void    channel_send_open(int);
@@ -177,7 +176,6 @@ void     channel_output_poll(void);
 
 int      channel_not_very_much_buffered_data(void);
 void     channel_close_all(void);
-void     channel_free_all(void);
 int      channel_still_open(void);
 char   *channel_open_message(void);
 int     channel_find_open(void);

diff --git a/serverloop.c b/serverloop.c
index d7282fe2a266f738e930ff41a87390bbcf980686..8a82af55b7bac6f87ccbaf29754aac4dfd21b886 100644 (file)
--- a/serverloop.c
+++ b/serverloop.c
@@ -35,7 +35,7 @@
  */
 
 #include "includes.h"
-RCSID("$OpenBSD: serverloop.c,v 1.80 2001/10/09 19:51:18 markus Exp $");
+RCSID("$OpenBSD: serverloop.c,v 1.81 2001/10/09 21:59:41 markus Exp $");
 
 #include "xmalloc.h"
 #include "packet.h"
@@ -674,6 +674,7 @@ server_loop2(Authctxt *authctxt)
        fd_set *readset = NULL, *writeset = NULL;
        int rekeying = 0, max_fd, status, nalloc = 0;
        pid_t pid;
+       sigset_t oset, nset;
 
        debug("Entering interactive session for SSH2.");
 
@@ -696,12 +697,17 @@ server_loop2(Authctxt *authctxt)
                        channel_output_poll();
                wait_until_can_do_something(&readset, &writeset, &max_fd,
                    &nalloc, 0);
+
+               /* block SIGCHLD while we check for dead children */
+               sigemptyset(&nset);
+               sigaddset(&nset, SIGCHLD);
+               sigprocmask(SIG_BLOCK, &nset, &oset);
                if (child_terminated) {
                        while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
                                session_close_by_pid(pid, status);
-                       /* XXX race */
                        child_terminated = 0;
                }
+               sigprocmask(SIG_SETMASK, &oset, NULL);
                if (!rekeying)
                        channel_after_select(readset, writeset);
                process_input(readset);
@@ -709,35 +715,21 @@ server_loop2(Authctxt *authctxt)
                        break;
                process_output(writeset);
        }
-       /* close all channels, no more reads and writes */
-       channel_close_all();
-
        if (readset)
                xfree(readset);
        if (writeset)
                xfree(writeset);
 
-       mysignal(SIGCHLD, SIG_DFL);
+       /* free all channels, no more reads and writes */
+       channel_free_all();
 
-       /* collect dead children */
+       /* collect remaining dead children, XXX not necessary? */
+       mysignal(SIGCHLD, SIG_DFL);
        while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
                session_close_by_pid(pid, status);
-       /*
-        * there is a race between channel_detach_all() killing remaining
-        * children and children dying before kill()
-        */
-       channel_detach_all();
-
-       while (session_have_children()) {
-               pid = waitpid(-1, &status, 0);
-               if (pid > 0)
-                       session_close_by_pid(pid, status);
-               else {
-                       error("waitpid returned %d: %s", pid, strerror(errno));
-                       break;
-               }
-       }
-       channel_free_all();
+
+       /* close remaining sessions, e.g remove wtmp entries */
+       session_close_all();
 }
 
 static void

diff --git a/session.c b/session.c
index c1305da6acba8a851b4251dedf697b95e8f4b807..0d6ebdaeafe91f248a6b5f6bcd3cdb8ba1ca1d9c 100644 (file)
--- a/session.c
+++ b/session.c
@@ -33,7 +33,7 @@
  */
 
 #include "includes.h"
-RCSID("$OpenBSD: session.c,v 1.105 2001/10/09 19:32:49 markus Exp $");
+RCSID("$OpenBSD: session.c,v 1.106 2001/10/09 21:59:41 markus Exp $");
 
 #include "ssh.h"
 #include "ssh1.h"
@@ -1949,22 +1949,6 @@ session_close_by_pid(pid_t pid, int status)
        session_close(s);
 }
 
-int
-session_have_children(void)
-{
-       int i;
-
-       for(i = 0; i < MAX_SESSIONS; i++) {
-               Session *s = &sessions[i];
-               if (s->used && s->pid != -1) {
-                       debug("session_have_children: id %d pid %d", i, s->pid);
-                       return 1;
-               }
-       }
-       debug("session_have_children: no more children");
-       return 0;
-}
-
 /*
  * this is called when a channel dies before
  * the session 'child' itself dies
@@ -1982,15 +1966,29 @@ session_close_by_channel(int id, void *arg)
        s->chanid = -1;
 
        debug("session_close_by_channel: channel %d kill %d", id, s->pid);
-       if (s->pid == 0) {
-               /* close session immediately */
-               session_close(s);
-       } else {
-               /* notify child, delay session cleanup */
-               if (kill(s->pid, (s->ttyfd == -1) ? SIGTERM : SIGHUP) < 0)
+       if (s->pid != 0) {
+               /* notify child */
+               if (kill(s->pid, SIGHUP) < 0)
                        error("session_close_by_channel: kill %d: %s",
                            s->pid, strerror(errno));
        }
+       session_close(s);
+}
+
+void
+session_close_all(void)
+{
+       int i;
+       for(i = 0; i < MAX_SESSIONS; i++) {
+               Session *s = &sessions[i];
+               if (s->used) {
+                       if (s->chanid != -1) {
+                               channel_cancel_cleanup(s->chanid);
+                               s->chanid = -1;
+                       }
+                       session_close(s);
+               }
+       }
 }
 
 static char *

diff --git a/session.h b/session.h
index a04fa6f2bcc95589f32e32f8532c0375e9502b59..d2b0d9364096094667fcca4e7d48b8772d8771ae 100644 (file)
--- a/session.h
+++ b/session.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: session.h,v 1.11 2001/07/02 13:59:15 markus Exp $     */
+/*     $OpenBSD: session.h,v 1.12 2001/10/09 21:59:41 markus Exp $     */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -32,6 +32,6 @@ int    session_open(Authctxt*, int);
 void    session_input_channel_req(int, void *);
 void    session_close_by_pid(pid_t, int);
 void    session_close_by_channel(int, void *);
-int     session_have_children(void);
+void    session_close_all(void);
 
 #endif