diff --git a/misc/buffer-tests/HandleTests_Common.cc b/misc/buffer-tests/HandleTests_Common.cc index 1af955b..87b958c 100755 --- a/misc/buffer-tests/HandleTests_Common.cc +++ b/misc/buffer-tests/HandleTests_Common.cc @@ -262,33 +262,139 @@ static void Test_Detach_Does_Not_Change_Standard_Handles() { // Detaching the current console does not affect the standard handles. printTestName(__FUNCTION__); auto check = [](Worker &p) { - Handle h[] = { p.getStdin(), p.getStdout(), p.getStderr() }; + auto handles1 = handleValues(stdHandles(p)); p.detach(); - CHECK(p.getStdin().value() == h[0].value()); - CHECK(p.getStdout().value() == h[1].value()); - CHECK(p.getStderr().value() == h[2].value()); + auto handles2 = handleValues(stdHandles(p)); + CHECK(handles1 == handles2); }; // Simplest form of the test. - Worker p1; - check(p1); + { + Worker p1; + check(p1); + } // Also do a test with duplicated handles, just in case detaching resets // the handles to their defaults. - Worker p2; - p2.getStdin().dup(TRUE).setStdin(); - p2.getStdout().dup(TRUE).setStdout(); - p2.getStderr().dup(TRUE).setStderr(); - check(p2); + { + Worker p2; + p2.getStdin().dup(TRUE).setStdin(); + p2.getStdout().dup(TRUE).setStdout(); + p2.getStderr().dup(TRUE).setStderr(); + check(p2); + } + // Do another test with STARTF_USESTDHANDLES, just in case detaching resets + // to the hStd{Input,Output,Error} values. + { + Worker p3; + auto pipe = newPipe(p3, true); + SpawnParams sp(true); + sp.sui.dwFlags |= STARTF_USESTDHANDLES; + sp.sui.hStdInput = std::get<0>(pipe).value(); + sp.sui.hStdOutput = std::get<1>(pipe).value(); + sp.sui.hStdError = std::get<1>(pipe).dup(true).value(); + auto p3c = p3.child(sp); + check(p3c); + } } static void Test_Activate_Does_Not_Change_Standard_Handles() { - // Setting a console as active does not change the standard handles. + // SetConsoleActiveScreenBuffer does not change the standard handles. + // MSDN documents this fact on "Console Handles"[1] + // + // "Note that changing the active screen buffer does not affect the + // handle returned by GetStdHandle. Similarly, using SetStdHandle to + // change the STDOUT handle does not affect the active screen buffer." + // + // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682075.aspx printTestName(__FUNCTION__); Worker p; - auto out = p.getStdout(); - auto err = p.getStderr(); + auto handles1 = handleValues(stdHandles(p)); p.newBuffer(TRUE).activate(); - CHECK(p.getStdout().value() == out.value()); - CHECK(p.getStderr().value() == err.value()); + auto handles2 = handleValues(stdHandles(p)); + CHECK(handles1 == handles2); +} + +static void Test_Active_ScreenBuffer_Order() { + // SetActiveConsoleScreenBuffer does not increase a refcount on the + // screen buffer. Instead, when the active screen buffer's refcount hits + // zero, Windows activates the most-recently-activated buffer. + + auto firstChar = [](Worker &p) { + auto h = p.openConout(); + auto ret = h.firstChar(); + h.close(); + return ret; + }; + + printTestName(__FUNCTION__); + { + // Simplest test + Worker p; + p.getStdout().setFirstChar('a'); + auto h = p.newBuffer(false, 'b').activate(); + h.close(); + CHECK_EQ(firstChar(p), 'a'); + } + { + // a -> b -> c -> b -> a + Worker p; + p.getStdout().setFirstChar('a'); + auto b = p.newBuffer(false, 'b').activate(); + auto c = p.newBuffer(false, 'c').activate(); + c.close(); + CHECK_EQ(firstChar(p), 'b'); + b.close(); + CHECK_EQ(firstChar(p), 'a'); + } + { + // a -> b -> c -> b -> c -> a + Worker p; + p.getStdout().setFirstChar('a'); + auto b = p.newBuffer(false, 'b').activate(); + auto c = p.newBuffer(false, 'c').activate(); + b.activate(); + b.close(); + CHECK_EQ(firstChar(p), 'c'); + c.close(); + CHECK_EQ(firstChar(p), 'a'); + } +} + +static void Test_GetStdHandle_SetStdHandle() { + // A commenter on the Old New Thing blog suggested that + // GetStdHandle/SetStdHandle could have internally used CloseHandle and/or + // DuplicateHandle, which would have changed the resource management + // obligations of the callers to those APIs. In fact, the APIs are just + // simple wrappers around global variables. Try to write tests for this + // fact. + // + // http://blogs.msdn.com/b/oldnewthing/archive/2013/03/07/10399690.aspx#10400489 + printTestName(__FUNCTION__); + auto &hv = handleValues; + { + // Set values and read them back. We get the same handles. + Worker p; + auto pipe = newPipe(p); + auto rh = std::get<0>(pipe); + auto wh1 = std::get<1>(pipe); + auto wh2 = std::get<1>(pipe).dup(); + setStdHandles({ rh, wh1, wh2 }); + CHECK(hv(stdHandles(p)) == hv({ rh, wh1, wh2})); + + // Call again, and we still get the same handles. + CHECK(hv(stdHandles(p)) == hv({ rh, wh1, wh2})); + } + { + Worker p; + p.getStdout().setFirstChar('a'); + p.newBuffer(false, 'b').activate().setStdout().dup().setStderr(); + std::get<1>(newPipe(p)).setStdout().dup().setStderr(); + + // SetStdHandle doesn't close its previous handle when it's given a new + // handle. Therefore, the two handles given to SetStdHandle for STDOUT + // and STDERR are still open, and the new screen buffer is still + // active. + CHECK_EQ(p.openConout().firstChar(), 'b'); + } } void runCommonTests() { @@ -301,4 +407,6 @@ void runCommonTests() { Test_Input_Vs_Output(); Test_Detach_Does_Not_Change_Standard_Handles(); Test_Activate_Does_Not_Change_Standard_Handles(); + Test_Active_ScreenBuffer_Order(); + Test_GetStdHandle_SetStdHandle(); } diff --git a/misc/buffer-tests/HandleTests_Traditional.cc b/misc/buffer-tests/HandleTests_Traditional.cc index c321543..47f1648 100755 --- a/misc/buffer-tests/HandleTests_Traditional.cc +++ b/misc/buffer-tests/HandleTests_Traditional.cc @@ -10,7 +10,7 @@ static void checkAttachHandleSet(Worker &child, Worker &source) { auto cvecInherit = inheritableHandles(cvec); auto svecInherit = inheritableHandles(source.scanForConsoleHandles()); auto hv = &handleValues; - if (hv(cvecInherit) == hv(svecInherit) && hv(cvecInherit) == hv(cvec)) { + if (hv(cvecInherit) == hv(svecInherit) && allInheritable(cvec)) { return; } source.dumpConsoleHandles(); @@ -36,13 +36,11 @@ static void Test_NewConsole_Resets_Everything() { Handle::invent(h.value(), p).close(); } - // XXX: On Win8+, I doubt we can expect particular console handle values... - // XXX: Actually, I'm not sure how much of this test is valid there at all... auto checkClean = [](Worker &proc) { proc.dumpConsoleHandles(); - CHECK(proc.getStdin().uvalue() == 0x3); - CHECK(proc.getStdout().uvalue() == 0x7); - CHECK(proc.getStderr().uvalue() == 0xb); + CHECK_EQ(proc.getStdin().uvalue(), 0x3u); + CHECK_EQ(proc.getStdout().uvalue(), 0x7u); + CHECK_EQ(proc.getStderr().uvalue(), 0xbu); auto handles = proc.scanForConsoleHandles(); CHECK(handleValues(handles) == (std::vector { proc.getStdin().value(), diff --git a/misc/buffer-tests/harness/RemoteHandle.cc b/misc/buffer-tests/harness/RemoteHandle.cc index 1f45ad2..ffbdbdb 100755 --- a/misc/buffer-tests/harness/RemoteHandle.cc +++ b/misc/buffer-tests/harness/RemoteHandle.cc @@ -203,3 +203,24 @@ std::vector handleValues(const std::vector &vec) { } return ret; } + +// It would make more sense to use a std::tuple here, but it's inconvenient. +std::vector stdHandles(RemoteWorker &worker) { + return { + worker.getStdin(), + worker.getStdout(), + worker.getStderr(), + }; +} + +// It would make more sense to use a std::tuple here, but it's inconvenient. +void setStdHandles(std::vector handles) { + ASSERT(handles.size() == 3); + handles[0].setStdin(); + handles[1].setStdout(); + handles[2].setStderr(); +} + +bool allInheritable(const std::vector &vec) { + return handleValues(vec) == handleValues(inheritableHandles(vec)); +} diff --git a/misc/buffer-tests/harness/RemoteHandle.h b/misc/buffer-tests/harness/RemoteHandle.h index 099b55b..04a03d2 100755 --- a/misc/buffer-tests/harness/RemoteHandle.h +++ b/misc/buffer-tests/harness/RemoteHandle.h @@ -71,3 +71,6 @@ std::vector inheritableHandles( const std::vector &vec); std::vector handleInts(const std::vector &vec); std::vector handleValues(const std::vector &vec); +std::vector stdHandles(RemoteWorker &worker); +void setStdHandles(std::vector handles); +bool allInheritable(const std::vector &vec);