Various crashes when using QNetworkAccessManager

testus

I have a QDialog class that opens a URL on clicking of a button.
This is my implementation:

ImageExplorer::ImageExplorer(QWidget *parent) :
	QDialog(parent),
	ui(new Ui::ImageExplorer){
	ui->setupUi(this);    

	managerPHP = new QNetworkAccessManager(this);
	connect(managerPHP, SIGNAL(finished(QNetworkReply*)), this, SLOT(openedPHP()) );
	argument = "QString with some argument";
}

void ImageExplorer::on_buttonUpload_clicked(){    
	QNetworkRequest req;
	req.setUrl(QUrl("http://example.com/test.php?name="+argument));
	req.setRawHeader( "User-Agent" , "Mozilla Firefox" );
	managerPHP->get(req);
}

void ImageExplorer::openedPHP(){
	qDebug()<< "called php";
}

ImageExplorer::~ImageExplorer(){
	delete ui;
	delete managerPHP;
}

My problem is that my application will crash 8 out of 10 times if I close the QDialog after the upload button was clicked. If the button wasn't clicked it doesn't crash. Sometimes it will also crash immediately after clicking the button but most of the times it will crash when closing the QDialog.

What am I doing wrong in my implementation?

koahnig

Hi and welcome to devnet

You have

    ui->setupUi(this);

and you delete ui explicitely below

ImageExplorer::~ImageExplorer(){
    delete ui;
    delete managerPHP;
}

The first statement (setupUi) hands the ownership of ui to your object. In the destructor you delete explicitely, but it will be also when your object is destroyed. You use basically the traditional way with the explicit delete and the Qt handling.
Remove the "delete ui" and you are probably fine.

testus

@koahnig
Thank you for you answer. Unfortunately, your suggestion did not change anything which I kind of expected since what you suggested did not have anything to do with the QNetworkAccessManager. Again, I can create and close the QDialog a hundred times and it will never crash. Only if managerPHP->get(req); gets called it will result in a crash when I close the QDialog.

koahnig

Sorry, the same is true for managerPHP. Remove this delete as well in your destructor.

testus

@koahnig
The same problem persists. It will crash after closing the dialog.

koahnig

@testus
Theoretically there might be still a signal on its way, while you are destructing your object. However, I consider this as not possible.
Did you do rebuild?
To be save you can add before rebuild a qmake run. That makes sure that all is uptodate.

testus

@koahnig
I did a clean build. I do wait until I receive the message from openedPHP() before I close the QDialog. Sometimes it will work and I can click the button and close the QDialog afterwards without a crash. But thats quite rare. Other times it will immediately crash when on_buttonUpload_clicked() is called. But like I said, most times it crashes after closing the QDialog after having clicked the button.

koahnig

@testus

I do not see another problem in your code. Possibly it is in another part than the routines you are posting.
With similar destructors as you implemented with cleaning up the the pointer I faced also unmotivated crash, which I could not explain. So, if you delete other pointers to Qt objects as you do here, you better clean this up. Check out this description of deletelater()

Chris Kawa

As @koahnig mentioned there's not much here that could cause a crash. I suspect it's something in the code that creates and shows that dialog. Can you post this part?

I have also two comments to this discussion:

Contrary to what @koahnig said you should definitely delete the ui like in your original post! The ui is not a QObject and its lifetime is not managed by Qt parent-child mechanism.
The comment about deleting managerPHP is true though. You're giving it a parent (this), so don't delete it manually as this will crash the app.
You are leaking memory. The openedPHP method should take a QNetworkReply* parameter and call deleteLater() on it. It is not deleted automatically.

koahnig

@Chris-Kawa
You are right about ui. I was just following the destructors I have in some code. There ui is no pointer and therefore, there is no delete required.

Has this been changed lately?
Those classes have been created with designer, but some time back on Qt 4.

Chris Kawa

@koahnig You can set the wizard behavior in QtCreator in Tools -> Options -> C++ -> Qt Class Generation. Visual Studio add-in gives you this option in class wizard dialog. I moved to Qt5 a long time ago but I vaguely remember the default was the same around Qt 4.7ish. Not sure if it changed somewhere earlier.

Chris Kawa

This post is deleted!

koahnig

@Chris-Kawa
OK, thanks for advise. I had checked in the mean time with Qt 5.3 and designer does also do a pointer assignment now.
I have started years ago with MSVC and addin. I guess those dialogs are from that time. Never seen this possibility (at least I do not remember).
Anyway the thing is the generated code has to work and it does either way. Unfortunately, I step into this trap and giving wrong advise.