From: ian AT cygnus DOT com (Ian Lance Taylor) Subject: Re: Compatibility function for cygwin and a telnet problem 16 Feb 1998 13:21:19 -0800 Message-ID: <199802162037.PAA20554.cygnus.cygwin32.developers@subrogation.cygnus.com> References: <01BD3A60 DOT F41B8E80 AT sos> Reply-To: cygwin32-developers AT cygnus DOT com To: Sergey Okhapkin Cc: cgf AT bbc DOT com, cygwin32-developers AT cygnus DOT com From: Sergey Okhapkin Date: Sun, 15 Feb 1998 22:27:51 +0300 Christopher Faylor wrote: > Sergey, does login echo correctly for you when you telnet into an NT system? > Has anyone else noticed this problem? The scenario is the following: 1. Parent opens master side of pty. 2. Parent forks. 3. Child opens slave side of pty. With new Ian's algorhytm original ttyp->input_handle and output_handle are closed and the new ones are owned by child now. 4. WriteFile(ttyp->output_handle,...) in parent's doecho() fails because child's handles are unavailable for parent :-( Ian, do something, please... OK, I give up on making the pipe read work correctly. I changed do_output to loop using PeekNamedPipe and check whether the pipe had been closed. Please try this patch. Ian Index: tty.h =================================================================== RCS file: /cvs/cvsfiles/devo/winsup/tty.h,v retrieving revision 1.4.14.4 diff -u -r1.4.14.4 tty.h --- tty.h 1998/01/23 00:50:07 1.4.14.4 +++ tty.h 1998/02/16 20:36:02 @@ -53,8 +53,6 @@ HWND hwnd; /* Console window handle tty belongs to */ DWORD master_pid; /* Win32 PID of tty master process */ - DWORD handle_pid; /* Win32 PID of owner of tty real I/O handles */ - int slave_handles; /* Count of open slaves in handle_pid + 1 */ HANDLE input_handle; /* Real I/O handles of master tty */ HANDLE output_handle; @@ -76,6 +74,7 @@ int connect_tty (int); void free_tty (int); tty *gettty(int n) {return &ttys[n];} + int getcount (int n) { return count[n]; } void terminate (); void init (); }; Index: tty.cc =================================================================== RCS file: /cvs/cvsfiles/devo/winsup/tty.cc,v retrieving revision 1.6.8.1.4.11 diff -u -r1.6.8.1.4.11 tty.cc --- tty.cc 1998/02/11 02:17:36 1.6.8.1.4.11 +++ tty.cc 1998/02/16 20:36:02 @@ -74,6 +74,8 @@ sid = 0; pgid = 0; hwnd = NULL; + input_handle = INVALID_HANDLE_VALUE; + output_handle = INVALID_HANDLE_VALUE; } void @@ -134,7 +136,13 @@ if (total < 0) small_printf ("free_tty: count < 0\n"); if (total == 0) - ttys[ttynum].init (); + { + if (ttys[ttynum].input_handle != INVALID_HANDLE_VALUE) + CloseHandle (ttys[ttynum].input_handle); + if (ttys[ttynum].output_handle != INVALID_HANDLE_VALUE) + CloseHandle (ttys[ttynum].output_handle); + ttys[ttynum].init (); + } termios_printf ("tty %d allocated %d times\n", ttynum, count[ttynum]); } @@ -343,8 +351,6 @@ /* Save our pid */ ttyp->master_pid = GetCurrentProcessId (); - ttyp->handle_pid = ttyp->master_pid; - ttyp->slave_handles = 0; /* Allow the others to open us (for handle duplication) */ @@ -675,12 +681,47 @@ } if (rlen > sizeof outbuf) rlen = sizeof outbuf; + + HANDLE handle = m->get_input_handle (); - if (ReadFile (m->get_input_handle (), outbuf, rlen, &n, NULL) == FALSE) + /* Doing a busy wait like this is quite inefficient, but nothing + else seems to work completely. Windows should provide some sort + of overlapped I/O for pipes, or something, but it doesn't. */ + DWORD avail; + while (1) { - return 0; + if (! PeekNamedPipe (handle, NULL, 0, NULL, &avail, NULL)) + { + if (GetLastError () == ERROR_BROKEN_PIPE) + return 0; + __seterrno (); + return -1; + } + termios_printf ("%u available; %d handles\n", (int) avail, + s->t.getcount (m->ttynum)); + if (avail > 0) + break; + if (s->t.getcount (m->ttynum) == 1) + { + /* We have the only remaining open handle to this pty, which + we treat as EOF. */ + termios_printf ("all other handles closed\n"); + return 0; + } + termios_printf ("sleeping waiting for data\n"); + Sleep (10); } + + if (ReadFile (handle, outbuf, rlen, &n, NULL) == FALSE) + { + if (GetLastError () == ERROR_BROKEN_PIPE) + return 0; + __seterrno (); + return -1; + } + termios_printf ("len=%u\n", n); + if (ttyp->termios.c_lflag & FLUSHO) { ttyp->write_retval = n; @@ -758,11 +799,16 @@ while (1) { n = do_output ((fhandler_pty_master *) &master, buf, OUT_BUFFER_SIZE); - if (n == 0) + if (n < 0) { termios_printf ("ReadFile error %d\n", GetLastError ()); ExitThread (0); } + if (n == 0) + { + /* End of file. */ + ExitThread (0); + } n = master.f->write ((void *) buf, (size_t) n); ttyp->write_retval = n == -1? -get_errno (): n; SetEvent (master.output_done_event); @@ -849,36 +895,7 @@ sprintf (buf, IOCTL_DONE_EVENT, ttynum); ioctl_done_event = OpenEvent (EVENT_ALL_ACCESS, TRUE, buf); - /* Duplicate tty handles. The handles are held by the process with - ID ttyp->handle_pid. If that is the master, as determined by the - slave_handles field, then we use DUPLICATE_CLOSE_SOURCE to close - the handles in the master process. That is so that when we close - our descriptor, and thus our end of the pipes, the master will no - longer be able to read from or write to its end of the pipes. - - FIXME: This doesn't work if the slave pty is closed and then - reopened while the master pty remains open. To make it work in - this case, the master would have to keep track of whether there - are any slaves, and recreate the pipes when there were none. - - FIXME: This needs locking, as does most access to the shared tty - fields. */ - - DWORD access, options; - if (ttyp->slave_handles == 0) - { - termios_printf ("copying handles from master (%d)\n", - ttyp->master_pid); - access = PROCESS_ALL_ACCESS; - options = DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE; - } - else - { - termios_printf ("copying handles from slave (%d)\n", - ttyp->handle_pid); - access = PROCESS_DUP_HANDLE; - options = DUPLICATE_SAME_ACCESS; - } + /* Duplicate tty handles. */ if (ttyp->input_handle == INVALID_HANDLE_VALUE || ttyp->output_handle == INVALID_HANDLE_VALUE) @@ -889,7 +906,8 @@ return 0; } - HANDLE handle_owner = OpenProcess (access, FALSE, ttyp->handle_pid); + HANDLE handle_owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE, + ttyp->master_pid); if (handle_owner == NULL) { termios_printf ("can't open tty handle process\n"); @@ -900,7 +918,8 @@ HANDLE nh; if (!DuplicateHandle (handle_owner, ttyp->input_handle, - GetCurrentProcess (), &nh, 0, TRUE, options)) + GetCurrentProcess (), &nh, 0, TRUE, + DUPLICATE_SAME_ACCESS)) { termios_printf ("can't duplicate input (%d)\n", GetLastError ()); __seterrno (); @@ -910,7 +929,7 @@ set_handle (nh); if (!DuplicateHandle (handle_owner, ttyp->output_handle, GetCurrentProcess (), &output_handle_, 0, TRUE, - options)) + DUPLICATE_SAME_ACCESS)) { termios_printf ("can't duplicate output (%d)\n", GetLastError ()); __seterrno (); @@ -919,21 +938,6 @@ } CloseHandle (handle_owner); - if (ttyp->slave_handles == 0) - { - /* We are now the owner of the handles, and anybody who wants to - open the pty needs to copy them from us, since they are now - closed in the master. FIXME: Needs locking. */ - ttyp->input_handle = get_handle (); - ttyp->output_handle = output_handle_; - ttyp->handle_pid = GetCurrentProcessId (); - /* We store the number of open slaves + 1 in slave_handles, so - that we can always tell where the handles are. */ - ttyp->slave_handles = 2; - } - else if (ttyp->handle_pid == GetCurrentProcessId ()) - ++ttyp->slave_handles; - termios_printf("tty%d opened\n", ttynum); return this; @@ -971,24 +975,6 @@ termios_printf ("CloseHandle 6 failed (%d)\n", GetLastError ()); tty *ttyp = s->t.gettty (ttynum); - if (ttyp->handle_pid == GetCurrentProcessId ()) - { - --ttyp->slave_handles; - /* We store the number of open slaves + 1 in slave_handles, so - if it drops to 1 we know that all the slaves are closed. */ - if (ttyp->slave_handles <= 1 - && ttyp->input_handle != INVALID_HANDLE_VALUE - && ttyp->handle_pid != ttyp->master_pid) - { - if (! CloseHandle (ttyp->input_handle)) - termios_printf ("CloseHandle 7 failed (%d)\n", GetLastError ()); - if (! CloseHandle (ttyp->output_handle)) - termios_printf ("CloseHandle 8 failed (%d)\n", GetLastError ()); - ttyp->input_handle = INVALID_HANDLE_VALUE; - ttyp->output_handle = INVALID_HANDLE_VALUE; - } - } - detach_tty (ttynum); termios_printf ("tty%d closed\n", ttynum); @@ -1210,6 +1196,11 @@ memcpy (buf, (char *) &output_mutex, sizeof (output_mutex)); buf += sizeof (output_mutex); + /* We call attach_tty here to increment the count of references to + this tty. This will have to be changed if attach_tty ever + worries about just which process has the tty attached. */ + attach_tty (ttynum); + len += this->fhandler_base::linearize (buf); return len; } @@ -1231,8 +1222,6 @@ memcpy ((char *) &output_mutex, buf, sizeof (output_mutex)); buf += sizeof (output_mutex); - attach_tty (ttynum); - int len = buf - orig_buf; return (len + this->fhandler_base::de_linearize (buf)); } @@ -1305,9 +1294,6 @@ fts->set_handle (nh); - if (ttyp->slave_handles != 0 && ttyp->handle_pid == GetCurrentProcessId ()) - ++ttyp->slave_handles; - return 0; err: @@ -1510,8 +1496,6 @@ /* Save our pid */ ttyp->master_pid = GetCurrentProcessId (); - ttyp->handle_pid = ttyp->master_pid; - ttyp->slave_handles = 0; /* Allow the others to open us (for handle duplication) */ @@ -1594,19 +1578,6 @@ detach_tty (ttynum); - tty *ttyp = s->t.gettty (ttynum); - if (ttyp->slave_handles == 0 - && ttyp->input_handle != INVALID_HANDLE_VALUE - && ttyp->sid == 0) - { - if (! CloseHandle (ttyp->input_handle)) - termios_printf ("CloseHandle 6 failed (%d)\n", GetLastError ()); - if (! CloseHandle (ttyp->output_handle)) - termios_printf ("CloseHandle 7 failed (%d)\n", GetLastError ()); - ttyp->input_handle = INVALID_HANDLE_VALUE; - ttyp->output_handle = INVALID_HANDLE_VALUE; - } - termios_printf("tty%d closed\n", ttynum); return 0; @@ -1642,9 +1613,11 @@ len--; } n = do_output (this, cptr, len); + if (n < 0) + return -1; if (output_done_event != NULL) SetEvent (output_done_event); - if (pktmode) + if (pktmode && n > 0) n++; return n; } @@ -1735,7 +1708,12 @@ buf += sizeof (output_mutex); memcpy (buf, (char *) &pktmode, sizeof (pktmode)); buf += sizeof (pktmode); - + + /* We call attach_tty here to increment the count of references to + this tty. This will have to be changed if attach_tty ever + worries about just which process has the tty attached. */ + attach_tty (ttynum); + len += this->fhandler_base::linearize (buf); return len; } @@ -1756,8 +1734,6 @@ buf += sizeof (output_mutex); memcpy ((char *) &pktmode, buf, sizeof (pktmode)); buf += sizeof (pktmode); - - attach_tty (ttynum); int len = buf - orig_buf; return (len + this->fhandler_base::de_linearize (buf));