QNetworkReply network reply timeout helper

Question

Since Qt still does not support to set timeouts on QNetworkRequest objects, I wrote this little wrapper class:

/**
Usage:
new QReplyTimeout(yourReply, msec);
When the timeout is reached the given reply is closed if still running
*/
class QReplyTimeout : public QObject {
 Q_OBJECT
public:
 QReplyTimeout(QNetworkReply* reply, const int timeout) : QObject(reply) {
 Q_ASSERT(reply);
 if (reply) {
 QTimer::singleShot(timeout, this, SLOT(timeout()));
 }
 }
private slots:
 void timeout() {
 QNetworkReply* reply = static_cast<QNetworkReply*>(parent());
 if (reply->isRunning()) {
 reply->close();
 }
 }
};

You can use it in a very simple fire-and-forget manner:

QNetworkAccessManager networkAccessManger;
QNetworkReply* reply = networkAccessManger.get(QNetworkRequest(QUrl("https://www.google.com")));
new QReplyTimeout(r, 100);

If the call to Google does not finish in 100ms, it is aborted. And since the QReplyTimeout class is parented to the QNetworkReply, it will be destroyed automatically.

Review the code for any pitfalls, memory leaks, invalid casts and if it's generally in a good style.

• user23573: class is quick and concise. I see no problems there.
• CodingWithSpike: I think this could only be used to shorten the timeout, not lengthen it. If you had a large file upload, I believe the internal Qt timeout would still trigger a QNetworkReply.TimeoutError

Answer 1

Here are my thoughts:

1. It's a simple class that allocates quite a few times on the heap. You could minimize the number of implicit allocations it does. The QTimer::singleShot creates a temporary QObject instance and allocates a bunch on the heap. You can avoid it by handling a single-shot timer explicitly using timerEvent. You also avoid the need to set up any connections that way. A QObject does about two allocations when the first connection is added to it.

2. Use Qt-5 style connections, but that doesn't apply anymore in light of #1 above.

3. Add a static helper method that uses this class to set a timeout on a network request. The fact that you need to allocate ReplyTimeout is an implementation detail of sorts that could be abstracted out that way.

4. Check if the reply is running before you set a timeout on it. Perhaps it was an incorrect/impossible request at the moment it was submitted to the manager and it was immediately finished.

5. You're not supposed to name your classes with a Q prefix. It's reserved for Qt.

The code below is portable across Qt 4 and Qt 5:

class ReplyTimeout : public QObject {
 Q_OBJECT
 QBasicTimer m_timer;
public:
 ReplyTimeout(QNetworkReply* reply, const int timeout) : QObject(reply) {
 Q_ASSERT(reply);
 if (reply && reply->isRunning())
 m_timer.start(timeout, this);
 }
 static void set(QNetworkReply* reply, const int timeout) {
 new ReplyTimeout(reply, timeout);
 }
protected:
 void timerEvent(QTimerEvent * ev) {
 if (!m_timer.isActive() || ev->timerId() != m_timer.timerId())
 return;
 auto reply = static_cast<QNetworkReply*>(parent());
 if (reply->isRunning())
 reply->close();
 m_timer.stop();
 }
};

Use:

QNetworkAccessManager networkAccessManger;
QNetworkReply* reply = 
 networkAccessManger.get(QNetworkRequest(QUrl("https://www.google.com")));
ReplyTimeout::set(reply, 100);

• CaptRespect: This looks great, but if the upload/download is large won't this timeout even though it's transmitting? Should you cancel/restart the timer when uploadProgress is received?
• Violet Giraffe: @CaptRespect: I'd say that's outside the scope of responsibility of this class. It's the user's responsibility to specify a sensible timeout based on the knowledge about what exactly is being downloaded via the supplied QNetworkReply.
• Violet Giraffe: The Q_OBJECT macro is not required here and it hurts at least the compilation time (if not runtime weight of the object). Also, I would rename timeout to something like timeoutMsec so that it's immediately clear which units the timeout is in.
• Kuba hasn't forgotten Monica: The Q_OBJECT macro is required on all QObject-derived classes. It is an implementation detail that it could be skipped on some classes, but it makes the classes thus "improved" not be QObjects in the Liskov Substitution Principle sense. E.g. all users of QObject expect that the metadata is correct and thus qobject_cast works. Skip Q_OBJECT and it's not true, thus you have a class that the compiler lets you use where a QObject can be used, but it's not really a QObject anymore. As for the timeout, it's Qt lingo for ms. If anything, I'd use std::duration instead.