Implementing a timeout with QNetworkAccessManager

Jez1337

Hello, can somebody please explain this weird behaviour when using QNetworkAccessManager and a local event loop.

My function uses QNetworkAccessManager to check if a URL is available. It uses a local event loop to cancel the request if it takes longer than 30 seconds.

Running on a Windows 10 machine which doesn't have OpenSSL installed, the reply should always give an error, eg QNetworkReply::SslHandshakeFailedError. What's actually happening is that if I comment out the local event loop, the request succeeds, which is correct because in reality the URL is available but shouldn't be happening because OpenSSL isn't installed. If the event loop is enabled (ie not commented out), the request fails, which is as expected, because OpenSSL isn't installed.

The event loop timer never runs out because I'm testing it on sites like www.google.com, but the mere prescence of the loop in my code seems to be affecting whether or not the request succeeds.

The question
Why on earth would commenting out the local event loop affect whether QNetworkReply returns an error or not? Have I completely misunderstood what the event loop is actually doing?

Code

bool resourceOk(const QUrl& url)
{
	std::unique_ptr<QNetworkAccessManager> networkManager;
	QNetworkRequest request;
	const int timeLimitInMilliSec = 30000;
	request.setUrl(url);

	QTimer timer;
	timer.setSingleShot(true);
	QEventLoop eventLoop;
	connect(networkManager.get(), SIGNAL(finished(QNetworkReply *)), &eventLoop, SLOT(quit()));
	connect(&timer, SIGNAL(timeout()), &eventLoop, SLOT(quit()));

	timer.start(timeLimitInMilliSec);
	QNetworkReply* reply = networkManager->get(request);
	eventLoop.exec();

	if (!timer.isActive())
	{
		disconnect(networkManager.get(), SIGNAL(finished(QNetworkReply *)), &eventLoop, SLOT(quit()));
		reply->abort();
		return false;
	}
	else
		timer.stop();

	switch (reply->error())
	{
	case QNetworkReply::NoError:
	{
		//We get here, but shouldn't when the above event loop code is commented out

		int statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
		//handling of the http status code here
		break;
	}
	case QNetworkReply::HostNotFoundError:
	{
		return false;
	}
	default:
	{
		//We should always end up here, because OpenSSL isn't available,
		//but we only do if the event loop is working
		return false;
	}
	}

	return true;
}

System details

Windows 10, Qt 5.13.2
Compiling in Visual Studio 2017, x64 Release mode

Thanks in advance!

Jez1337

Experimenting with various different computers and verisons of the code have revealed the answers:

Solution

Why does removing the event loop cause the code to work, when it should fail? Because my function never waits for the reply to finish before trying to handle the response. When the event loop is removed, the function jumps straight to handling the resonse, and apparently QNetworkReply::error() returns NoError by default, until such a time as it knows an error has occured. This makes it look like OpenSSL is working, when in fact it isn't even present on the system.

In hindsight, this is obvious when you look at the code without the event loop:

	std::unique_ptr<QNetworkAccessManager> networkManager;
	QNetworkRequest request;
	request.setUrl(url);

	QNetworkReply* reply = networkManager->get(request);

	switch (reply->error())
	{
		//Response handling here
	}

Miscellanea discovered while doing this

Many Qt Windows developers use the Shining Light version of OpenSSL. This installs a lot of stuff but all that seems to matter from there in terms of getting QNetworkAccessManager to work is to have libcrypto-1_1-x64.dll and libssl-1_1-x64.dll in one of the folders referenced by the PATH environment variable, as well as a certificates file pointed to by the SSL_CERT_FILE environment variable
When you uninstall the Shining Light setup of OpenSSL, it can leave its .dlls in your C:\Windows\System32 folder, making it look like it has been uninstalled when it is, in fact, still there
My local event loop is considered bad practice because it blocks the thread it is running in, but actually there was absolutely nothing wrong with my function. In my application which doesn't have any user interactivity, blocking the thread is a nonissue because nothing else is going on at the same time. More modern users of Qt can do away with QTimer entirely and use QNetworkRequest::setTransferTimout(), as pointed out by @artwaw

Thanks to everyone for their help, which undoubtedly allowed me to pin down the problem

Christian Ehrlicher

Don't use a local event loop. It's not needed.

Jez1337

Hi @Christian-Ehrlicher, many thanks for your fast reply. I'd like to remote the event loop but doing so raises two issues:

The target machine doesn't have OpenSSL, so removing the event loop seems be masking this fact. This appears to be incorrect behaviour. Do you know why it's happening?
The event loop is my attempt to avoid networkManager->get(request) hanging indefinitely. If I remove the event loop, what guarantee do I have that it won't sit waiting for the reply for ages?

Thanks in advance

SGaist

Hi,

@Jez1337 said in Implementing a timeout with QNetworkAccessManager:

The target machine doesn't have OpenSSL, so removing the event loop seems be masking this fact. This appears to be incorrect behaviour. Do you know why it's happening?

That may not be true. Your target machine may have OpenSSL made available in one of the folder listed in the PATH environment variable. You might even have a version installed in your system folder (happened to me with a computer with stuff pre-installed by the manufacturer). You should check that.

Jez1337

Hi @SGaist, thanks for jumping in, that actually did occur to me and would definitely explain the "weird" behaviour.

On Windows, the only way I've found to check for the presence of OpenSSL, is to enter something like openssl version in a console window. On my deployment machine, this returns an error, suggesting that OpenSSL isn't there. Do you know of another way to verify this please? I mean it's not sitting there in the C drive or anything obvious

Christian Ehrlicher

@Jez1337 said in Implementing a timeout with QNetworkAccessManager:

f I remove the event loop, what guarantee do I have that it won't sit waiting for the reply for ages?

A timer?

Jez1337

@Christian-Ehrlicher The above code uses QTimer. Is there a different way to implement it other than how I've done it?

Christian Ehrlicher

@Jez1337 said in Implementing a timeout with QNetworkAccessManager:

Is there a different way to implement it other than how I've done it?

Use another slot instead QEventLoop::quit() and do your timeout handling there.

Jez1337

@Christian-Ehrlicher That would be roughly like this I think?

resourceOk(const QUrl& url)
{
	timer.setSingleShot(true);
	connect(networkManager.get(), SIGNAL(finished(QNetworkReply *)), this, SLOT(finishedFine()));
	connect(&timer, SIGNAL(timeout()), this, SLOT(timedOut()));

	timer.start(timeLimitInMilliSec);
	QNetworkReply* reply = networkManager->get(request);
}

finishedFine()
{
	timer.stop();
	//handle reply here
}

timedOut()
{
	//handle what happened when the timer ran out
}

This gets rid of the event loop but creates a new problem, which is that anything calling resourceOk() then becomes event driven, and has to wait for signals depending on whether the download timed out or worked successfully, in order to know if the resource was ok or not.

Assuming I've not misunderstood what you're suggesting, this is going to create more complexity overall because instead of one local event loop in this function, I'll then have to either put in signals and slots or event loops any time resourceOk() is called. No?

Christian Ehrlicher

Otherwise resourceOk() will block the gui which is for sure not what you want. Local eventloops should be avoided.

artwaw

Hi,
out of curiosity: have you considered using QNetworkRequest::setTransferTimout()? Same method exists for QNetworkAccessManager - I haven't used it but from the looks of - might do the trick?

Jez1337

@Christian-Ehrlicher There isn't really a GUI in this application - everything runs sequentially on one thread so it's fine if that thread gets blocked. It isn't going to be doing anything until it knows if resourceOk returned true or not. That's why I was using a local event loop in the first place!

Jez1337

@artwaw Hello thank you for the suggestion! I was wondering why I didn't implement that - it's Qt 5.15 apparently and I'm in 5.13 and can't upgrade!

I'm guessing they implemented that precisely because of people like me posting questions about how to do this (rather obvious) operation using QTimer

Christian Ehrlicher

@Jez1337 said in Implementing a timeout with QNetworkAccessManager:

I'm guessing they implemented that precisely because of people like me posting questions about how to do this (rather obvious) operation using QTimer

But this is async too.

Again: no local eventloops.

artwaw

@Jez1337 In 5.13 you can have QNetworkError enum and error() signal in QNetworkReply class - you can connect to that. There is no need to break async.
Maybe it is worth to examine how you can manipulate timeout this way?

Jez1337

@artwaw do you mean when my timer expires, emit QNetworkReply::error(QNetworkReply::TimeoutError), and then use one error handler for everything? I think that would work as a similar alternative to what @Christian-Ehrlicher suggested.

Recap

Why does removing the event loop cause the code to work, when it should fail? Not sure yet. @SGaist suggested maybe OpenSSL is actually there, but I'm not sure how to check since it's not being found in the command prompt
We can remove the event loop entirely. This sidesteps the issue altogether, making the function more event driven. However I'm reluctant for two reasons:

a) I'm using the function as follows:

if (resourceOk("www.google.com"))
	//Do stuff
else
	//Do other stuff

I can't see any easy way to keep the interface this clean while also having resourceOk() use separate event handlers for different signals under the hood.
b) It doesn't matter if my function blocks, because it's not called at times where there is also a human user there mucking around in a GUI

Next steps:

If there is a way to keep using resourceOk() as above while using separate signals and slots under the hood, then I'll go ahead and remove the event loop
I'm going to try a search of the C drive for the dlls included in OpenSSL, to rule out the possibility that it is hiding in there somewhere and my code is actually working properly. If anybody knows a faster way of finding if OpenSSL was installed, please do say!

Jez1337

Experimenting with various different computers and verisons of the code have revealed the answers:

Solution

Why does removing the event loop cause the code to work, when it should fail? Because my function never waits for the reply to finish before trying to handle the response. When the event loop is removed, the function jumps straight to handling the resonse, and apparently QNetworkReply::error() returns NoError by default, until such a time as it knows an error has occured. This makes it look like OpenSSL is working, when in fact it isn't even present on the system.

In hindsight, this is obvious when you look at the code without the event loop:

	std::unique_ptr<QNetworkAccessManager> networkManager;
	QNetworkRequest request;
	request.setUrl(url);

	QNetworkReply* reply = networkManager->get(request);

	switch (reply->error())
	{
		//Response handling here
	}

Miscellanea discovered while doing this

Many Qt Windows developers use the Shining Light version of OpenSSL. This installs a lot of stuff but all that seems to matter from there in terms of getting QNetworkAccessManager to work is to have libcrypto-1_1-x64.dll and libssl-1_1-x64.dll in one of the folders referenced by the PATH environment variable, as well as a certificates file pointed to by the SSL_CERT_FILE environment variable
When you uninstall the Shining Light setup of OpenSSL, it can leave its .dlls in your C:\Windows\System32 folder, making it look like it has been uninstalled when it is, in fact, still there
My local event loop is considered bad practice because it blocks the thread it is running in, but actually there was absolutely nothing wrong with my function. In my application which doesn't have any user interactivity, blocking the thread is a nonissue because nothing else is going on at the same time. More modern users of Qt can do away with QTimer entirely and use QNetworkRequest::setTransferTimout(), as pointed out by @artwaw

Thanks to everyone for their help, which undoubtedly allowed me to pin down the problem

SGaist

@Jez1337 said in Implementing a timeout with QNetworkAccessManager:

When you uninstall the Shining Light setup of OpenSSL, it can leave its .dlls in your C:\Windows\System32 folder, making it look like it has been uninstalled when it is, in fact, still there

Are you sure their installer put the dlls in the system folder ? If so this is wrong and they should be notified about that issue.

JKSH

@Jez1337 said in Implementing a timeout with QNetworkAccessManager:

Many Qt Windows developers use the Shining Light version of OpenSSL. This installs a lot of stuff but all that seems to matter from there in terms of getting QNetworkAccessManager to work is to have libcrypto-1_1-x64.dll and libssl-1_1-x64.dll in one of the folders referenced by the PATH environment variable, as well as a certificates file pointed to by the SSL_CERT_FILE environment variable

When you uninstall the Shining Light setup of OpenSSL, it can leave its .dlls in your C:\Windows\System32 folder, making it look like it has been uninstalled when it is, in fact, still there

The current 64-bit MSI installer by Shining Light (Win64OpenSSL_Light-1_1_1i.msi) installs the DLLs into C:\Program Files\OpenSSL-Win64. As @SGaist said, no installer (aside from official Windows installers) should put anything into C:\Windows

Anyway, you can install OpenSSL directly from the Qt online installer nowadays.

Jez1337

@JKSH , @SGaist You're both correct that the Shining Light installer doesn't put anything in C:\Windows\System32, unless you explicitly tell it to by checking the option during the installation process to do exactly that!

I avoid using the Qt online installer where possible but I think it was a good idea for them to include it so thank you for pointing that out. Having to install OpenSSL separately was always a faff, especially when an OpenSSL stack is included in the Chromium browser engine anyway