Fix [bda99f2393]. Windows non-blocking gets stdin truncation. See ticket for resolution.

2 files changed, 54 insertions, 28 deletions

diff --git a/tests/winConsole.test b/tests/winConsole.test
index 3597fe3..e87c870 100644
--- a/tests/winConsole.test
+++ b/tests/winConsole.test
@@ -155,7 +155,7 @@ test console-input-2.1 {Console file channel: non-blocking read} -constraints {
 } -result abc
 
 test console-input-3.0 {Console gets blocking - long lines bug-bda99f2393} -constraints {
-    win interactive bug-bda99f2393
+    win interactive
 } -body {
     prompt "Try typing a line of at least 256 characters. Hit ENTER exactly once unless you don't see another prompt.\n"
     gets stdin line
@@ -164,7 +164,7 @@ test console-input-3.0 {Console gets blocking - long lines bug-bda99f2393} -cons
 } -result {1 1 1}
 
 test console-input-3.1 {Console gets blocking, small channel buffer size - long lines bug-bda99f2393} -constraints {
-    win interactive bug-bda99f2393
+    win interactive
 } -body {
     prompt "Try typing a line of at least 256 characters. Hit ENTER exactly once unless you don't see another prompt.\n"
     set bufSize [fconfigure stdin -buffersize]
@@ -176,7 +176,7 @@ test console-input-3.1 {Console gets blocking, small channel buffer size - long
 } -result {1 1 1}
 
 test console-input-3.2 {Console gets nonblocking - long lines bug-bda99f2393} -constraints {
-    win interactive bug-bda99f2393
+    win interactive
 } -body {
     prompt "Try typing a line of at least 256 characters. Hit ENTER exactly once unless you don't see another prompt.\n"
     fconfigure stdin -blocking 0
@@ -189,7 +189,7 @@ test console-input-3.2 {Console gets nonblocking - long lines bug-bda99f2393} -c
 } -result {1 1 1}
 
 test console-input-3.3 {Console gets nonblocking small channel buffer size - long lines bug-bda99f2393} -constraints {
-    win interactive bug-bda99f2393
+    win interactive
 } -body {
     prompt "Try typing a line of at least 256 characters. Hit ENTER exactly once unless you don't see another prompt.\n"
     set bufSize [fconfigure stdin -buffersize]
diff --git a/win/tclWinConsole.c b/win/tclWinConsole.c
index eb81370..acd5851 100644
--- a/win/tclWinConsole.c
+++ b/win/tclWinConsole.c
@@ -70,14 +70,23 @@
 static int gInitialized = 0;
 
 /*
- * Permit CONSOLE_BUFFER_SIZE to be defined on build command for stress test.
- *
+ * INPUT_BUFFER_SIZE is size of buffer passed to ReadConsole in bytes.
+ * Note that ReadConsole will only allow reading of line lengths up to the
+ * max of 256 and buffer size passed to it. So dropping this below 512
+ * means user can type at most 256 chars.
+ */
+#ifndef INPUT_BUFFER_SIZE
+#define INPUT_BUFFER_SIZE 8192 /* In bytes, so 4096 chars */
+#endif
+
+/*
+ * CONSOLE_BUFFER_SIZE is size of storage used in ring buffers.
  * In theory, at least sizeof(WCHAR) but note the Tcl channel bug
  * https://core.tcl-lang.org/tcl/tktview/b3977d199b08e3979a8da970553d5209b3042e9c
  * will cause failures in test suite if close to max input line in the suite.
  */
 #ifndef CONSOLE_BUFFER_SIZE
-#define CONSOLE_BUFFER_SIZE 8000 /* In bytes */
+#define CONSOLE_BUFFER_SIZE 8192 /* In bytes */
 #endif
 
 /*
@@ -1143,15 +1152,22 @@ ConsoleInputProc(
 
 	/*
 	 * Blocking read. Just get data from directly from console. There
-	 * is a small complication in that we can only read even number
-	 * of bytes (wide-character API) and the destination buffer should be
-	 * WCHAR aligned. If either condition is not met, we defer to the
-	 * reader thread which handles these case rather than dealing with
+	 * is a small complication in that
+	 * 1. The destination buffer should be WCHAR aligned.
+	 * 2. We can only read even number of bytes (wide-character API).
+	 * 3. Caller has large enough buffer (else length of line user can
+	 *    enter will be limited)
+	 * If any condition is not met, we defer to the
+	 * reader thread which handles these cases rather than dealing with
 	 * them here (which is a little trickier than it might sound.)
+	 *
+	 * TODO - not clear this block is a useful optimization. bufSize by
+	 * default is 4K which is < INPUT_BUFFER_SIZE and will rarely be
+	 * increased on stdin.
 	 */
 	if ((1 & (size_t)bufPtr) == 0 /* aligned buffer */
-	    && bufSize > 1         /* Not single byte read */
-	) {
+	    && (1 & bufSize) == 0     /* Even number of bytes */
+	    && bufSize > INPUT_BUFFER_SIZE) {
 	    DWORD lastError;
 	    Tcl_Size numChars;
 	    ReleaseSRWLockExclusive(&handleInfoPtr->lock);
@@ -1630,9 +1646,11 @@ ConsoleReaderThread(
 {
     ConsoleHandleInfo *handleInfoPtr = (ConsoleHandleInfo *) arg;
     ConsoleHandleInfo **iterator;
-    char inputChars[200]; /* Temporary buffer */
     Tcl_Size inputLen = 0;
     Tcl_Size inputOffset = 0;
+    Tcl_Size lastReadSize = 0;
+    DWORD sleepTime;
+    char inputChars[INPUT_BUFFER_SIZE];
 
     /*
      * Keep looping until one of the following happens.
@@ -1666,7 +1684,6 @@ ConsoleReaderThread(
 		Tcl_Size nStored;
 
 		assert((inputLen - inputOffset) > 0);
-
 		nStored = RingBufferIn(&handleInfoPtr->buffer,
 				       inputOffset + inputChars,
 				       inputLen - inputOffset,
@@ -1713,21 +1730,27 @@ ConsoleReaderThread(
 	    continue;
 	}
 
+	assert(inputLen == 0);
+
 	/*
-	 * Both shared buffer and private buffer are empty. Need to go get
-	 * data from console but do not want to read ahead because the
-	 * interp thread might change the read mode, e.g. turning off echo
-	 * for password input. So only do so if at least one interpreter has
-	 * requested data.
+	 * Read more data in two cases:
+	 * 1. The previous read filled the buffer and there could be more
+	 *    data in the console internal *text* buffer. Note
+	 *    ConsolePendingInput (checked in ConsoleDataAvailable) will NOT
+	 *    show this. It holds input events not yet translated to text.
+	 * 2. Tcl threads want more data AND there is data in the
+	 *    ConsolePendingInput buffer. The latter check necessary because
+	 *    we do not want to read ahead because the interp thread might
+	 *    change the read mode, e.g. turning off echo for password
+	 *    input. So only do so if at least one interpreter has requested
+	 *    data.
 	 */
-	if ((handleInfoPtr->flags & CONSOLE_DATA_AWAITED)
-	    && ConsoleDataAvailable(handleInfoPtr->console)) {
+	if (lastReadSize == sizeof(inputChars)
+	    || ((handleInfoPtr->flags & CONSOLE_DATA_AWAITED)
+		&& ConsoleDataAvailable(handleInfoPtr->console))) {
 	    DWORD error;
 	    /* Do not hold the lock while blocked in console */
 	    ReleaseSRWLockExclusive(&handleInfoPtr->lock);
-	    /*
-	     * Note - the temporary buffer serves two purposes. It
-	     */
 	    error = ReadConsoleChars(handleInfoPtr->console,
 				     (WCHAR *)inputChars,
 				     sizeof(inputChars) / sizeof(WCHAR),
@@ -1735,17 +1758,21 @@ ConsoleReaderThread(
 	    AcquireSRWLockExclusive(&handleInfoPtr->lock);
 	    if (error == 0) {
 		inputLen *= sizeof(WCHAR);
-	    } else {
+		lastReadSize = inputLen;
+	    }
+	    else {
 		/*
 		 * We only store the last error. It is up to channel
 		 * handlers whether to close or not in case of errors.
 		 */
+		lastReadSize = 0;
 		handleInfoPtr->lastError = error;
 		if (handleInfoPtr->lastError == ERROR_INVALID_HANDLE) {
 		    handleInfoPtr->console = INVALID_HANDLE_VALUE;
 		}
 	    }
-	} else {
+	}
+	else {
 	    /*
 	     * Either no one was asking for data, or no data was available.
 	     * In the former case, wait until someone wakes us asking for
@@ -1753,7 +1780,6 @@ ConsoleReaderThread(
 	     * poll since ReadConsole does not support async operation.
 	     * So sleep for a short while and loop back to retry.
 	     */
-	    DWORD sleepTime;
 	    sleepTime =
 		handleInfoPtr->flags & CONSOLE_DATA_AWAITED ? 50 : INFINITE;
 	    SleepConditionVariableSRW(&handleInfoPtr->consoleThreadCV,