Some errors if I don't click on QMessageBox button

[jiveaxe]

Hi,
first of all sorry if I report in a new thread a question made in this discussion (http://www.qtcentre.org/forum/f-qt-programming-2/t-add-check-for-updates-feature-11246.html) but the problem not seems strictly related to that particular case. The problems shows when I don't click any button in QMessageBox asking if install (in the sample code a simple QFile::copy()) a downloaded file: in this case pops up the same dialog because is emitted a new QHTTP::requestFinished() signal. If then I click ok/cancel on these QMessageBoxes I got segmentation fault and/or


ASSERT: "!isEmpty()" in file ../../include/QtCore/../../src/corelib/tools/qlist.h, line 248

If I click on the confirmation massage without pausing or remove the qmessagebox and install just after downloading no errors are reported.

Test yourself this code (sorry if no so minimal):


#include <QApplication>
#include <QDialog>
#include <QtGui>
#include <QtNetwork>

#include <iostream>

class Dialog: public QDialog
{
Q_OBJECT

public:
Dialog(QWidget *parent = 0);

private slots:
void httpRequestFinished(int requestId, bool error);
void updateDataReadProgress(int bytesRead, int totalBytes);
void readResponseHeader(const QHttpResponseHeader &responseHeader);
void cancelDownload();
void downloadFile();

private:
QString fileName;
QFile *file;
QPushButton *downloadButton;
QPushButton *closeButton;
QProgressDialog *progressDialog;
QHttp *http;
int httpGetId;
QDir dir;
bool httpRequestAborted;
void installFile();
};

Dialog::Dialog(QWidget *parent)
: QDialog(parent)
{
downloadButton = new QPushButton("Download");
closeButton = new QPushButton("Close");

QHBoxLayout *buttonsLayout = new QHBoxLayout;
buttonsLayout->addStretch();
buttonsLayout->addWidget(downloadButton);
buttonsLayout->addWidget(closeButton);
buttonsLayout->addStretch();

setLayout(buttonsLayout);

connect(closeButton, SIGNAL(clicked()), this, SLOT(close()));
connect(downloadButton, SIGNAL(clicked()), this, SLOT(downloadFile()));

progressDialog = new QProgressDialog(this);
http = new QHttp(this);

connect(http, SIGNAL(requestFinished(int, bool)),
this, SLOT(httpRequestFinished(int, bool)));
connect(http, SIGNAL(dataReadProgress(int, int)),
this, SLOT(updateDataReadProgress(int, int)));
connect(http, SIGNAL(responseHeaderReceived(const QHttpResponseHeader &)),
this, SLOT(readResponseHeader(const QHttpResponseHeader &)));
connect(progressDialog, SIGNAL(canceled()), this, SLOT(cancelDownload()));
}

void Dialog::downloadFile()
{
QUrl url("http://www.nih.at/libzip/index.html");
QFileInfo fileInfo(url.path());
QString fileName;
fileName = QDir::tempPath() + fileInfo.fileName();

if (QFile::exists(fileName)) {
if (QMessageBox::question(this,
tr("HTTP"),
trUtf8("There already exists a file called %1 in the current directory. Overwrite?")
.arg(fileName),
QMessageBox::Ok|QMessageBox::Cancel, QMessageBox::Cancel)
== QMessageBox::Cancel)
return;
QFile::remove(fileName);
}
file = new QFile(fileName);
if (!file->open(QIODevice::WriteOnly)) {
QMessageBox::information(this,
tr("HTTP"),
trUtf8("Impossibile salvare il file %1: %2.")
.arg(fileName).arg(file->errorString()));
delete file;
file = 0;
return;
}
QHttp::ConnectionMode mode = url.scheme().toLower() == "https" ? QHttp::ConnectionModeHttps : QHttp::ConnectionModeHttp;
http->setHost(url.host(), mode, url.port() == -1 ? 0 : url.port());

httpRequestAborted = false;
httpGetId = http->get(url.path(), file);

progressDialog->setWindowTitle(tr("HTTP"));
progressDialog->setLabelText(tr("Downloading %1.").arg(fileName));
downloadButton->setEnabled(false);
}

void Dialog::cancelDownload()
{
httpRequestAborted = true;
http->abort();
downloadButton->setEnabled(true);
}

void Dialog::httpRequestFinished(int requestId, bool error)
{
std::cout << "httpRequestFinished()" << std::endl;
if (requestId != httpGetId)
return;
if (httpRequestAborted) {
if (file) {
file->close();
file->remove();
delete file;
file = 0;
}

progressDialog->hide();
return;
}

if (requestId != httpGetId)
return;

progressDialog->hide();
file->close();

if (error) {
file->remove();
QMessageBox::information(this,
tr("HTTP"),
trUtf8("Download failed: %1.")
.arg(http->errorString()));
} else
installFile();

delete file;
file = 0;
}

void Dialog::readResponseHeader(const QHttpResponseHeader &responseHeader)
{
if (responseHeader.statusCode() != 200) {
QMessageBox::information(this,
tr("HTTP"),
trUtf8("Download failed: %1.").
arg(responseHeader.reasonPhrase()));
httpRequestAborted = true;
progressDialog->hide();
http->abort();
return;
}
}

void Dialog::updateDataReadProgress(int bytesRead, int totalBytes)
{
if (httpRequestAborted)
return;

progressDialog->setMaximum(totalBytes);
progressDialog->setValue(bytesRead);
}

void Dialog::installFile()
{
if (QMessageBox::question(this,
tr("Ready"),
trUtf8("If you don't click me quickly\nI will make some unwanted thing."),
QMessageBox::Ok|QMessageBox::Cancel, QMessageBox::Cancel)
== QMessageBox::Ok) {
QString fileName;
QFileInfo fi(file->fileName());
fileName = fi.fileName();
file->copy(QDir::tempPath() + "copied-index.html");
}
downloadButton->setEnabled(true);
}

int main(int argc, char *argv[])
{
QApplication app(argc,argv);
Dialog *dialog = new Dialog;
dialog->show();
return app.exec();
}

#include "main.moc"

[jpn]

I recall seeing this bug but I couldn't find it on Task-Tracker. The problem is that QHttp::requestFinished() is emitted twice when the slot starts an event loop (a modal message box in your case). Here's a minimal compilable example reproducing the problem:


#include <QtCore>
#include <QtNetwork>

class Downloader : public QHttp
{
Q_OBJECT
public:
Downloader()
{
connect(this, SIGNAL(requestFinished(int,bool)), this, SLOT(onRequestFinished(int,bool)));
}

private slots:
void onRequestFinished(int id, bool error)
{
qDebug() << "Downloader::requestFinished()" << id << error;
if (id == 2)
{
// start an event loop when "get" request is finished
// NOTICE: "Downloader::requestFinished() 2 false" is printed twice
QEventLoop eventLoop;
eventLoop.exec();
}
}
};

int main(int argc, char* argv[])
{
QCoreApplication app(argc, argv);
Downloader downloader;
downloader.setHost("www.trolltech.com"); // id == 1
downloader.get("/index.html"); // id == 2
return app.exec();
}

#include "main.moc"

[jiveaxe]

I recall seeing this bug but I couldn't find it on Task-Tracker.

This means that the updater has to install new files soon after downloading them, quitting the mainapp without warnings and without let the user finish its work. Very bad solution.

Thanks

[jpn]

This means that the updater has to install new files soon after downloading them, quitting the mainapp without warnings and without let the user finish its work. Very bad solution.
Don't you notice how two message boxes appear on top of each other? Just add a proper check to work it around...

[jiveaxe]

Don't you notice how two message boxes appear on top of each other? Just add a proper check to work it around...

I have naively admit I don't undertand your tip; if you mean putting a control for avoiding the second messabox appear I have already tryed in this manner: I have created a bool installationPending which is set to true when the installFile() member is called and this bool is tested at the beginning of Dialog::httpRequestFinished(): the first time this bool is false so all the code of httpRequestFinished() is executed until calling installFile() which sets to true installationPending; so when QHttp::requestFinished() is emittend for the second time the code inside httpRequestFinished() is ignored, so no extra messagebox, but when I click "Ok" I got the usual:


ASSERT: "!isEmpty()" in file ../../include/QtCore/../../src/corelib/tools/qlist.h, line 248
Aborted (core dumped)

The real problem is not the second messagebox itself but the bug you talk about and the only solution seems to be or click "Ok" quite soon or make it install automatically without prompting.

Regards

[jpn]

A quick and dirty fix is to make installFile() a slot and in httpRequestFinished() invoke the slot with help of timer (to allow control to return to QHttp, where the signal was emitted from):


if (error) {
...
} else {
//installFile();
QTimer::singleShot(0, this, SLOT(installFile()));
}

//delete file;
//file = 0;

Notice also that you can't either delete the file here because you will later use it in installFile().

[jpn]

Yet another (easier and more elegant) way would be to simply force a queued connection:


connect(http, SIGNAL(requestFinished(int, bool)),
this, SLOT(httpRequestFinished(int, bool)), Qt::QueuedConnection);

In this case you wouldn't need modifications represented in my last message.

[jiveaxe]

Great code, jpn. Now the program runs very fine.

A thousand thanks.