How to synchronize QNetworkAccessManager ?

ChrisW67

Bug filed? Why?

This example:

meets the original brief
starts downloading once the application is displayed
does not do overtly non-painting things in paintEvent() to achieve that
does not require threads

#include "widget.h"
#include <QApplication>
int main(int argc, char *argv[]) {
    QApplication a(argc, argv);
    Widget w;
    w.show();
    return a.exec();
}

#ifndef WIDGET_H
#define WIDGET_H

#include "imageproc.h"

#include <QTextEdit>

class Widget : public QTextEdit
{
    Q_OBJECT

public:
    Widget(QWidget *parent = nullptr);
    ~Widget();

private slots:
    void startWhenDisplayed();
    void listStart(int imageCount);
    void imageProcessed(const QString &summary);
    void listComplete();

private:
    ImageProc *m_imgProc;
};
#endif // WIDGET_H

#include "widget.h"

#include <QTimer>

Widget::Widget(QWidget *parent)
    : QTextEdit(parent)
    , m_imgProc(new ImageProc(this))
{
    resize(640, 480);

    connect(m_imgProc, &ImageProc::listStart,      this, &Widget::listStart);
    connect(m_imgProc, &ImageProc::imageProcessed, this, &Widget::imageProcessed);
    connect(m_imgProc, &ImageProc::listComplete,   this, &Widget::listComplete);

    // Eliminates the rubbish with the paintEvent()
    QTimer::singleShot(0, this, &Widget::startWhenDisplayed);
}

Widget::~Widget() {}

void Widget::startWhenDisplayed() {
    setText("Starting image list fetch...\n");
    m_imgProc->fetchImageList(QUrl("http://localhost:8000/list.txt"));
}

void Widget::listStart(int imageCount) {
    append(QString("  Received list of %1 urls\n").arg(imageCount));
}

void Widget::imageProcessed(const QString &summary) {
    append(QString("  Received and processed image: %1\n").arg(summary));
}

void Widget::listComplete() {
    append("... completed.");
}

#ifndef IMAGEPROC_H
#define IMAGEPROC_H

#include <QObject>
#include <QNetworkAccessManager>
#include <QNetworkReply>
#include <QUrl>
#include <QImage>

class ImageProc : public QObject {
    Q_OBJECT

public:
    explicit ImageProc(QObject *parent = nullptr);
    void fetchImageList(const QUrl &url);

signals:
    void listStart(int imageCount);
    void imageProcessed(const QString &summary);
    void listComplete();

private slots:
    void replyReceived(QNetworkReply *reply);

private:
    void fetchNextImage();
    void processImage(QImage image);

private:
    QNetworkAccessManager *m_nam;
    bool m_haveList;
    QList<QUrl> m_urlList;
};

#endif // IMAGEPROC_H

#include "imageproc.h"


ImageProc::ImageProc(QObject *parent)
    : QObject(parent)
    , m_nam(new QNetworkAccessManager(this))
    , m_haveList(false)
    , m_urlList()
{
    connect(m_nam, &QNetworkAccessManager::finished, this, &ImageProc::replyReceived);
    // connect error handler etc...
}

void ImageProc::fetchImageList(const QUrl &url)
{
    QNetworkReply *reply = m_nam->get(QNetworkRequest(url));
    Q_UNUSED(reply);
}

void ImageProc::replyReceived(QNetworkReply *reply)
{
    if (reply->error() == QNetworkReply::NoError) {
        if (m_haveList) {
            // should be receiving an image
            QByteArray data = reply->readAll();
            QImage image;
            image.loadFromData(data);
            bool ok = image.loadFromData(data);
            if (ok)
                processImage(image);
        }
        else {
            // receiving a list
            QTextStream stream(reply);
            QString url;
            while (stream.readLineInto(&url)) {
                m_urlList.append(QUrl(url));
            }
            m_haveList = true;
            emit listStart(m_urlList.count());
        }

        // either way try to fetch an image
        fetchNextImage();
    }

    reply->deleteLater();
}

void ImageProc::fetchNextImage()
{
    if (m_urlList.isEmpty()) {
        m_haveList = false;
        emit listComplete();
        return;
    }

    QUrl url = m_urlList.takeFirst();
    QNetworkReply *reply = m_nam->get(QNetworkRequest(url));
    Q_UNUSED(reply);
}

void ImageProc::processImage(QImage image)
{
    QString summary = QString("%1x%2 depth %3")
                          .arg(image.width())
                          .arg(image.height())
                          .arg(image.depth());
    emit imageProcessed(summary);
}

QT       += core gui network

greaterThan(QT_MAJOR_VERSION, 4): QT += widgets

CONFIG += c++17

# You can make your code fail to compile if it uses deprecated APIs.
# In order to do so, uncomment the following line.
#DEFINES += QT_DISABLE_DEPRECATED_BEFORE=0x060000    # disables all the APIs deprecated before Qt 6.0.0

SOURCES += \
    main.cpp \
    imageproc.cpp \
    widget.cpp

HEADERS += \
    imageproc.h \
    widget.h

# Default rules for deployment.
qnx: target.path = /tmp/$${TARGET}/bin
else: unix:!android: target.path = /opt/$${TARGET}/bin
!isEmpty(target.path): INSTALLS += target

When served this list and images:

$ cat list.txt 
http://localhost:8000/image1.jpg
http://localhost:8000/image2.jpg
http://localhost:8000/image3.jpg
http://localhost:8000/image4.jpg
http://localhost:8000/image5.jpg

$ identify *.jpg
image1.jpg JPEG 100x100 100x100+0+0 8-bit sRGB 7166B 0.000u 0:00.000
image2.jpg JPEG 200x100 200x100+0+0 8-bit sRGB 12689B 0.000u 0:00.000
image3.jpg JPEG 300x100 300x100+0+0 8-bit sRGB 18501B 0.000u 0:00.000
image4.jpg JPEG 400x100 400x100+0+0 8-bit sRGB 23201B 0.000u 0:00.000
image5.jpg JPEG 500x100 500x100+0+0 8-bit sRGB 28794B 0.000u 0:00.000

Does this:

Would have worked 13 years ago.

uani

Because it does not work for him 13 years ago and it does not work for me last week under the same circumstances and is remedied by the same remedy.
As such there is either a bug or a documentation shortcoming.
In the bug filed I already stated the issue does not occur for the most trivial of cases such as yours thus it is likely no bug in general.
Using https, errors occur which are not delivered to sslErrors, this might be a contributor to the observed behavior, the cause is yet to be identified.

Christian Ehrlicher

As long as you can't provide a minimal compileable example the bug is on your side. Prove me wrong.

uani

well, likely not. not much can be done wrong using QNAM, particularly after having combed through the documentation. I found many StackOverflow posts exist mentioning an unidentified issue. Eg. https://stackoverflow.com/questions/2776640/qt-qnetworkaccessmanager-does-not-emit-signals . Identifying the real culprit from those posts is difficult. One such SO post let surmise QNAM is created in a paintEvent (a globe map service url was used). I managed to find this qt.io post. The issue existed, the issue exists. Something triggers the issue in Qt. (Or it is by design but still something adds to it.) I haven't found it yet. Of course, until you haven't proof -- and even if you have -- it's up to your desire and interest to look into QNAM.

ChrisW67

@uani said in How to synchronize QNetworkAccessManager ?:

Identifying the real culprit from those posts is difficult.

Yes, and this thread is no different. Diagnosing the problem with your code requires understanding the entire problem space and eliminating possible causes from most likely to least likely. If you, the OP in this thread, and the poster in your link will not engage with that process then you are going to have to live with the result.

uani

No. QNAM and slot connections have no multiple paths in the documentation.

Don‘t dare to write about my engagement.

Everything relevant has been stated.

Only reply if your to be reply contributes to a possible solution.

artwaw

I can't say I appreciate your attitude.
having said that - it would not hurt if you read with a modicum of understanding. If more experienced developers tell you that your design pattern is substandard then perhaps take it at the face value?

I used to deal with a use case where I had to design and write client-facing program able to manipulate pdf documents over the WebDAV protocol, enabling users to download and display/print hundreds of small, 1-2 paged files at the time.
I did it all using QNAM since it is more than sufficient for the task.
Thinks to consider:

QNAM has a limit for simultaneous tasks being processed, which - as stated - is six.
That means exactly what it says. You can queue/fire 500 downloads in one series but they will be processed in batches of six.
As QNAM is asynchronous you can't expect downloads to arrive in order they were called. Obviously, that depends on many factors but my experience just confirms that.
Idea of calling QNAM from the paintEvent is pure bonkers. That's not what it was designed to do in the event chain event.
If you don't have any other trigger (system event, any other async signal/slot to rely on) at least put the fetch loop on timer slot with a second or so delay. Timer can be started from the MainWindow constructor as the last entry, after QNAM and all the connections to it have been done. This is to allow main loop to spin up properly.
if you need to cross-reference requested URIs with the downloaded data (for the purpose of assigning proper file names let's say) you have to keep track of that yourself. The simplest way is QStringList or QStringListModel, where you can keep called URIs and then remove them once the finished() signal passes you reply object. You do have to remember to deleteLater() that reply. Also, in case of a huge number of files memory issues might arise (I remember stress testing my program with 500.000 files in one go, that's where disk cache for the model would come in handy - but YMMV).

uani

which design pattern?

"more experienced" : i refrain from details.

I have posted no code .

It has been established:

When paintEvent is not done QNAM signals no slots.
Whan paintEvent is done, QNAM signals slots.
An unknown factor elicits this. **

Thanks to me, Qt now knows* about this. It is up to their care and honor for their product to investigate this now or feel not so and wait.

I thank you for your points.

I read about point 1. The issue is not a matter of not receving everything. It is a matter of any reaction at all: timeout, error or data.

Point 4: the data is decided at paint time and the source set is too large to be in local space.

I have moved on and decoupled code for painting received data from requesting data already. The issue at hand is not one I need solving.

Trying to remember how I originally wrote the code i might have waited in a loop in paintEvent for threads to complete drawing to intermediate images.**

by the bug report which contains a path for documentation

** The app main event loop (I remember event loops are for signal and slots, I don't remember how they work with Qt's event system) might have been blocked by painting for signal processing which could explain the issue.

If ** is the reason, the bug is no bug and perhaps the documentation needs no update. This would have been something an attentive reader could have figured and not write haughty responses.

artwaw

Nobody here pays me (or anyone else here) to deal with your attitude. Best of luck.

Christian Ehrlicher

@uani said in How to synchronize QNetworkAccessManager ?:

It is up to their care and honor for their product to investigate this now or feel not so and wait

Again: as long as you can't provide a minimal, compilable example nothing will happen and the bug will get closed soon.

uani

@artwaw said in How to synchronize QNetworkAccessManager ?:

your attitude

I thanked you for your attempts at contributing help, Idiot !

your attitude. what a wickedness. what a disgrace .

@Christian-Ehrlicher said in How to synchronize QNetworkAccessManager ?:

Again: as long as you can't provide a minimal, compilable example nothing will happen and the bug will get closed soon.

Again: It is up to your care and honor for your product to investigate this now or feel not so and wait

Christian Ehrlicher

@uani said in How to synchronize QNetworkAccessManager ?:

I thanked you for your attempts at contributing help, Idiot !

your attitude. what a wickedness. what a disgrace .

Please follow the code of conduct: https://forum.qt.io/topic/145364/forum-guidelines-code-of-conduct

Your wording is inappropriate, please stop it.