Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReadConsole returns TRUE after CancelIoEx #17791

Open
alabuzhev opened this issue Aug 23, 2024 · 0 comments
Open

ReadConsole returns TRUE after CancelIoEx #17791

alabuzhev opened this issue Aug 23, 2024 · 0 comments
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Meta The product is the management of the products. Tracking-External This bug isn't resolved, but it's following an external workitem. zInbox-Bug Ignore me!

Comments

@alabuzhev
Copy link
Contributor

alabuzhev commented Aug 23, 2024

Windows Terminal version

Latest source

Windows build number

10.0.19045.4780

Other Software

No

Steps to reproduce

  1. Compile the code:
Code
#ifndef UNICODE
#define UNICODE
#endif

#ifndef _UNICODE
#define _UNICODE
#endif

#include <chrono>
#include <iostream>
#include <thread>
#include <string>

#include <windows.h>

using namespace std::literals;

int main()
{
	const auto In = GetStdHandle(STD_INPUT_HANDLE);

	std::thread Thread([&]
	{
		std::this_thread::sleep_for(3s);
		CancelIoEx(In, {});
	});

	std::wcout << L"Do not touch anything for about 3s" << std::endl;

	wchar_t Buffer[1024];
	DWORD NumberOfCharsRead{};
	const auto Result = ReadConsole(In, Buffer, ARRAYSIZE(Buffer), &NumberOfCharsRead, {});
	const auto Error = GetLastError();

	std::wcout << L"Result: " << Result << std::endl;
	std::wcout << L"Error:  " << Error << std::endl;
	std::wcout << L"Chars:  " << NumberOfCharsRead << std::endl;
	std::wcout << L"Data:   " << std::wstring(Buffer, NumberOfCharsRead) << std::endl;

	Thread.join();

	std::wcout << L"Now enter something: " << std::endl;

	if (ReadConsole(In, Buffer, ARRAYSIZE(Buffer), &NumberOfCharsRead, {}))
	{
		std::wcout << L"You have entered: " << std::wstring(Buffer, NumberOfCharsRead) << std::endl;
	}
}
  1. Run it (anywhere: WT, OpenConsole, conhost)
  2. Inspect the output

Expected Behavior

The program reads the console input in a blocking way.
If there is no input, another thread issues CancelIoEx after 3 seconds.

Since the ReadConsole did not read anything, it should return FALSE:

  • It is logical.
  • Even Raymond Chen says so:

    If you had used Read­File instead of fgets, the read would have failed with error code ERROR_OPERATION_ABORTED, as documented by Cancel­Io­Ex.

So the program should print Result: 0.

Actual Behavior

ReadConsole does not read anything, does not update NumberOfCharsRead, but returns TRUE.

It also leaves the input in somewhat inconsistent state, which you can see by typing something after the cancellation: the first input will be discarded. I think this was already mentioned here: #12143 (comment).

The incorrect return value is much worse though: it is a common pattern to leave NumberOfCharsRead uninitialized, because either ReadConsole succeeds and initializes it, or it fails and it makes no sense to look there anyway.
In the code above I initialized it, but if I didn't do so an uninitialized read would've occurred, from both NumberOfCharsRead and Buffer.

Notably the last error is correctly set to 995 - ERROR_OPERATION_ABORTED - "The I/O operation has been aborted because of either a thread exit or an application request.", but who checks the last error on successful calls?

@alabuzhev alabuzhev added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 23, 2024
@lhecker lhecker added Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Priority-1 A description (P1) labels Sep 3, 2024
@carlos-zamora carlos-zamora removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Sep 4, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Sep 4, 2024
@carlos-zamora carlos-zamora added Product-Meta The product is the management of the products. and removed Needs-Tag-Fix Doesn't match tag requirements labels Sep 4, 2024
@carlos-zamora carlos-zamora added this to the Terminal v1.23 milestone Sep 4, 2024
@carlos-zamora carlos-zamora added zInbox-Bug Ignore me! Tracking-External This bug isn't resolved, but it's following an external workitem. labels Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Meta The product is the management of the products. Tracking-External This bug isn't resolved, but it's following an external workitem. zInbox-Bug Ignore me!
Projects
None yet
Development

No branches or pull requests

3 participants