Getting Pixmap is a null pixmap on calling a function 500 times

Question

I am showing a image in qt label. Below is my code:

void MyClass::onPushButtonClicked(QString myurl)
{
    this->setCursor(Qt::WaitCursor);
    ui.qtImageLabel->clear();
    qDebug()<<QTime::currentTime()<<"MyClass: onPushButtonClicked";
    QNetworkAccessManager *qnam_push_button_clicked_show_image;
    QNetworkReply *reply;
    QNetworkRequest request;
    request.setHeader( QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded" );
    QUrl url(myurl);
    request.setUrl(url);
    qnam_push_button_clicked_show_image = new QNetworkAccessManager(this);
    if(qnam_push_button_clicked_show_image)
    {
        QObject::connect(qnam_push_button_clicked_show_image, SIGNAL(finished(QNetworkReply*)),
                         this, SLOT(onPushButtonClickedRequestCompleted(QNetworkReply*)));
        reply = qnam_push_button_clicked_show_image->post(request, url.encodedQuery());
        QEventLoop loop;
        QObject::connect(reply, SIGNAL(finished()), &loop, SLOT(quit()));
        loop.exec();
    }
}

void MyClass::onPushButtonClickedRequestCompleted(QNetworkReply *reply)
{
    qDebug()<<QTime::currentTime()<<"MyClass: onPushButtonClickedRequestCompleted request completed";
    if (reply->error() != QNetworkReply::NoError)
    {
        qDebug() << "Error in" << reply->url() << ":" << reply->errorString();
        this->setCursor(Qt::ArrowCursor);
        return;
    }
    QByteArray data = reply->readAll();
    QPixmap pixmap;
    pixmap.loadFromData(data);
    int width;
    int height;
    //application size can be changed
    QRect rec = QApplication::desktop()->screenGeometry();
    height = rec.height();
    width = rec.width();
    qDebug()<<QTime::currentTime()<<width<<","<<height;
    QSize *size = new QSize(width,height);
    if(size)
    {
        QPixmap scaledPixmap = pixmap.scaled(*size);
        ui.qtImageLabel->setPixmap(scaledPixmap);
    }
    if(size)
    {
        delete size;
        size = NULL;
    }
    data.clear();
    this->setCursor(Qt::ArrowCursor);
    reply->deleteLater();
    return;
}

On clicking push button It will send a request to server and will show a different image received from server. It is working fine if it does't exceeds 500 times. If it exceeds that first this error has been shown

QPixmap::scaled: Pixmap is a null pixmap

and it doesn't show the image. Then if someone again sends a request for an image then it shows the following error: Qt has caught an exception thrown from an event handler. Throwing exceptions from an event handler is not supported in Qt. You must re implement QApplication::notify() and catch all exceptions there.

I am not getting what is the error in the above code. Can someone please tell me how to solve this?

Answer 1

The obvious leak is qnam_push_button_clicked_show_image = new QNetworkAccessManager(this);, which doesn't have a balanced delete anywhere. QNAMs should typically created once, then reused for the lifetime of the application rather than created for a single request. So by turning qnam_push_button_clicked_show_image in a class member (same as ui) you'll fix both your leak and improve the efficiency of the code.

That said, I don't think that's what causes your QPixmap error. If you're running this code on X11, then QPixmap is backed by an X Pixmap resource, which is limited by various factors (software and hardware). Even though from your code there's no obvious leak, it could be that repeatedly allocating large pixmaps slowly fragments the memory pool managed by X, up to the point where it can't allocate a block large enough for the scaled pixmap and then triggers the error. Or it could be a driver bug somewhere in the graphics stack. Have you tried if changing the scaled size increases or decreases the limit before it starts breaking? If so, switching to QImage might help relieving the pressure on X.

Aside from that, the code could use some cleanup, especially that superfluous QEventLoop usage. I'm guessing it's a way to prevent the button from being clicked several times until the new image has been loaded, but I'd much rather implement this using button.setEnabled(false) while the image is downloading, because nested event loops combined with network events is a recipe for countless reentrancy issues and hard to debug crashes/bugs.

I'm also confused about why size is allocated on the heap , especially when it's deleted right after, and these if (size) are really confusing, as they can be understood as if (size->isValid()) while what they really mean is if (size != nullptr), which is pretty much guaranteed as the chance of getting an OOM on that line is infinitesimally low. (if you did eventually run out of memory, my guess is it would likely happen in the readAll() or loadFromData() calls above).

ps: good luck pressing that button another 500 times to check if fixing the leak helped ;)