Solved Implementing a timeout with QNetworkAccessManager
-
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!
-
-
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()
returnsNoError
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
-
-
Don't use a local event loop. It's not needed.
-
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
-
-
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.
-
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 -
@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?
-
@Christian-Ehrlicher The above code uses
QTimer
. Is there a different way to implement it other than how I've done it? -
@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.
-
@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? -
Otherwise resourceOk() will block the gui which is for sure not what you want. Local eventloops should be avoided.
-
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? -
@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!
-
@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
-
@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.
-
@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? -
@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 GUINext 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!
-
-
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()
returnsNoError
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
-
-
@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.
-
@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.
-
-
@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