About the cancellable SendMessageTimeout implementation

[gexgd0419]

Here's the code that makes SendMessageTimeout cancellable at any time, by separating the real SendMessageTimeout into multiple calls with shorter timeout, and check for cancellation after each return:

nvda/nvdaHelper/local/nvdaHelperLocal.cpp

Lines 90 to 103 in 45b2d1d

	for (UINT remainingTimeout = uTimeout; remainingTimeout > 0; remainingTimeout -= (remainingTimeout > CANCELSENDMESSAGE_CHECK_INTERVAL) ? CANCELSENDMESSAGE_CHECK_INTERVAL : remainingTimeout) {
	if (WaitForSingleObject(cancelCallEvent, 0) == WAIT_OBJECT_0) {
	// Note that cancellation is based on whether the *main* thread is alive.
	if (_notifySendMessageCancelled) {
	_notifySendMessageCancelled();
	}
	SetLastError(ERROR_CANCELLED);
	return 0;
	}
	if ((ret = real_SendMessageTimeoutW(hwnd, Msg, wParam, lParam, fuFlags, std::min(remainingTimeout, CANCELSENDMESSAGE_CHECK_INTERVAL), lpdwResult)) != 0 || GetLastError() != ERROR_TIMEOUT) {
	// Success or error other than timeout.
	return ret;
	}
	}

My concern is that this would send the same message multiple times if the application fails to process the message in this shorter period.

According to Raymond Chen's blog What happens to a sent message when SendMessageTimeout reaches its timeout, the sent message will be removed if the target fails to retrieve the message after timeout, but once the message is retrieved, the application is allowed to process the message until it completes. When the timeout is reached, SendMessageTimeout is allowed to return, while the target is still processing the message.

So if the application fails to meet the first timeout, this loop will just keep sending more messages until the target is determined to be "hung" by the system. The actual allowed timeout for the target application is CANCELSENDMESSAGE_CHECK_INTERVAL, which is 400, not the total timeout provided as an argument.

Most of the time, an application either responds in time, or becomes too laggy for users if it fails to respond in 400 milliseconds. Imagine if every keystroke has a 400ms delay. So maybe this wouldn't cause many problems pratically. I just wonder if it is a good design.

The blog actually mentioned a possible solution for this:

But wait, how do I know when the window procedure is done? The Send­Message­Timeout function doesn’t tell me! Yup, that’s right. If you need to do cleanup after message processing is complete, you should use the Send­Message­Callback function, which calls you back when the receiving thread completes message processing. When the callback fires, that’s when you do your cleanup.

We can use SendMessageCallback function instead. Although it doesn't accept a timeout, it won't block the caller thread, and when the target is done, you will be called back with the result. If you want to wait in the function, you can use a message loop: SendMessageCallback requires a message loop so that your callback can be called properly. This way, the message is only sent once, the target is allowed to use up the whole timeout period, and it will be able to react to cancellations more quickly, since it don't need to wait another interval to check for the event. MsgWaitForMultipleObjects can be helpful here.

Or is there something that prevents us from using SendMessageCallback that I'm not aware of?

cc. @jcsteh

[jcsteh]

So if the application fails to meet the first timeout, this loop will just keep sending more messages until the target is determined to be "hung" by the system.

That's true, but practically, if the app is hung, it's not going to process the message. There's a slight chance that we could race such that the app processes the message just as our timeout elapses, but that will generally only happen once. I agree it's not ideal, but it's difficult to avoid and far better than hanging NVDA.

SendMessageCallback requires a message loop so that your callback can be called properly.

This is the problem. A message loop also allows other things to run, which sometimes isn't expected and can cause re-entry, etc. I guess we could use SendMessageCallback if SMTO_BLOCK isn't set. But if SMTO_BLOCK is set, that means the caller isn't expecting to run other things while waiting.

it will be able to react to cancellations more quickly, since it don't need to wait another interval to check for the event.

In theory, this is true. In practice, the minimum cancellation timeout is 500 ms, so the user will be waiting a minimum of 500 ms anyway. 500 ms is obviously shorter than 800 ms, but at the point that we're recovering from a hang, I don't think this is a significant problem.

In short, you could experiment with using SendMessageCallback if SMTO_BLOCK isn't set, but we still need the old approach if SMTO_BLOCK is set.