Don't clear the software flow control flag before draining for last

close or assert the bug that it is clear when leaving.

Remove an unrelated rotted comment that was attached to the buggy
clearing.

Since draining is not done in more cases, flushing is needed in more
cases, so start fixing flushing:
- do a full flush in ttydisc_close().  State what POSIX requires more
  clearly.  This was missing ttydevsw_pktnotify() calls to tell the
  devsw layer to flush.  Hardware tty drivers don't actually flush
  since they don't understand this API.
- fix 2 missing wakeups in tty_flush().  Most of the wakeups here are
  unnecessary for last close.  But ttydisc_close() did one of the
  missing ones.

This flow control bug ameliorated the design bug of requiring
potentially unbounded waits in draining.  Software flow control is the
easiest way to get an unbounded wait, and a long wait is sometimes
actually useful.  Users can type the xoff character on the receiver
and (if ixon is set on the sender) expect the output to be held until
the user is ready for more.

Hardware flow control can also give the unbounded wait, and this bug
didn't affect hardware flow control.  Unbounded waits from hardware
flow control take a more unusual configuration.  E.g., a terminal
program that controls the modem status lines, or unplugging the cable
in a configuration where this doesn't break the connection.

The design bug is still ameliorated by a newer bug in draining for
last close -- the 1 second timeout.  E.g., if the user types the
xoff character and the sender reaches last close, then output is
not resumed and the wait times out after just 1 second.  This is
broken, but preferable to an unbounded wait.  Before this change,
the output was resumed immediately and usually completed.

Submitted by:	bde
MFC after:	2 weeks

2 files changed, 10 insertions, 15 deletions

diff --git a/sys/kern/tty.c b/sys/kern/tty.c
index 04a10b3..f10b9ea 100644
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -201,7 +201,6 @@ ttydev_leave(struct tty *tp)
 		constty_clear();
 
 	/* Drain any output. */
-	MPASS((tp->t_flags & TF_STOPPED) == 0);
 	if (!tty_gone(tp))
 		tty_drain(tp, 1);
 
@@ -352,11 +351,7 @@ ttydev_close(struct cdev *dev, int fflag, int devtype __unused,
 	if (fflag & FREVOKE)
 		tty_flush(tp, FWRITE);
 
-	/*
-	 * This can only be called once. The callin and the callout
-	 * devices cannot be opened at the same time.
-	 */
-	tp->t_flags &= ~(TF_EXCLUDE|TF_STOPPED);
+	tp->t_flags &= ~TF_EXCLUDE;
 
 	/* Properly wake up threads that are stuck - revoke(). */
 	tp->t_revokecnt++;
@@ -1456,12 +1451,15 @@ tty_flush(struct tty *tp, int flags)
 		tp->t_flags &= ~TF_HIWAT_OUT;
 		ttyoutq_flush(&tp->t_outq);
 		tty_wakeup(tp, FWRITE);
-		if (!tty_gone(tp))
+		if (!tty_gone(tp)) {
+			ttydevsw_outwakeup(tp);
 			ttydevsw_pktnotify(tp, TIOCPKT_FLUSHWRITE);
+		}
 	}
 	if (flags & FREAD) {
 		tty_hiwat_in_unblock(tp);
 		ttyinq_flush(&tp->t_inq);
+		tty_wakeup(tp, FREAD);
 		if (!tty_gone(tp)) {
 			ttydevsw_inwakeup(tp);
 			ttydevsw_pktnotify(tp, TIOCPKT_FLUSHREAD);
diff --git a/sys/kern/tty_ttydisc.c b/sys/kern/tty_ttydisc.c
index 63b144a..99fde31 100644
--- a/sys/kern/tty_ttydisc.c
+++ b/sys/kern/tty_ttydisc.c
@@ -94,14 +94,11 @@ ttydisc_close(struct tty *tp)
 	/* Clean up our flags when leaving the discipline. */
 	tp->t_flags &= ~(TF_STOPPED|TF_HIWAT|TF_ZOMBIE);
 
-	/* POSIX states we should flush when close() is called. */
-	ttyinq_flush(&tp->t_inq);
-	ttyoutq_flush(&tp->t_outq);
-
-	if (!tty_gone(tp)) {
-		ttydevsw_inwakeup(tp);
-		ttydevsw_outwakeup(tp);
-	}
+	/*
+	 * POSIX states that we must drain output and flush input on
+	 * last close.  Draining has already been done if possible.
+	 */
+	tty_flush(tp, FREAD | FWRITE);
 
 	if (ttyhook_hashook(tp, close))
 		ttyhook_close(tp);