Why QNetworkReply run-time crash on deleteLater() ?

[IMAN4K]

Hi everyone.
Please look at this piece of code for QNetworkReply slot:

void MyClass::someSlot(QNetworkReply* reply) {
		if (reply->error()) {
			log(QString("{%1(%2)} network error(%3)").arg(__FILE__).arg(__LINE__).arg(reply->errorString()));
			emit netError(reply->error(), reply->errorString());
			return;
		}

		const auto map = QJsonDocument::fromJson(reply->readAll()).object().toVariantMap();
		if (!map.find(QStringLiteral("success")).value().toBool()) {
			const auto msg = map.find(QStringLiteral("message")).value().toString();
			log(QString("{%1(%2)} API error(%3)").arg(__FILE__).arg(__LINE__).arg(msg));
			emit apiError(map.find(QStringLiteral("errorCode")).value().toInt(), msg);
			return;
		}

		const auto token = map.find(QStringLiteral("registration_token")).value().toString();
		const auto inviteURL = map.find(QStringLiteral("complete_invite_url")).value().toUrl();
		const auto pollingURL = map.find(QStringLiteral("polling_url")).value().toUrl();
		const auto timeOut = map.find(QStringLiteral("token_duration")).value().toInt();
		QString printerID;

		const auto items = QUrlQuery(pollingURL).queryItems();
		for (const auto id : items) {
			if (id.first == QStringLiteral("printerid"))
				printerID = id.second;
		}
		log(QString("anonymous registration successed (printerid:%1)").arg(printerID));
		emit registered(token, printerID, inviteURL, pollingURL, QTime(0, 0, 0).addSecs(timeOut));

		reply->deleteLater();
	}

Why i'm getting crash on reply->deleteLater(); ?

Doc clearly says about QNetworkAccessManager::finished(QNetworkReply *reply):

Note: Do not delete the reply object in the slot connected to this signal. Use deleteLater().

[raven-worx]

@IMAN4K
maybe because the reply is already deleted somewhere else?

btw. you have 2 return statements in your slot implementation which do not delete the reply object.

[IMAN4K]

@raven-worx
Hi.
deleted where ? i didn't deleted in my code
If it was suppose to be deleted somewhere else or QNetworkAccessManager takes the ownership, so why should we use deleteLater ?

Yes, should take care of that two returns.

[raven-worx]

@IMAN4K said in Why QNetworkReply run-time crash on deleteLater() ?:

deleted where ? i didn't deleted in my code

ok, then lets start over again.
What exactly makes you sure that deleteLater() causes the crash?
Can you post a callstack at the time of the crash?

[IMAN4K]

@raven-worx
Simple, if i remove reply->deleteLater() there's no crash!
someSlot is a lambda in initRegister function in my source code:

[J.Hilk]

@IMAN4K said in Why QNetworkReply run-time crash on deleteLater() ?:

someSlot is a lambda in initRegister function in my source code:

we're getting closer,

can you show us the whole code secetion/function, where the lambda is defined? You're most likely accessing the reply pointer there, somewhere, outside the lambda scope.

[IMAN4K]

Here's the interesting part:
api.h:

	class HTTPWorker : public QObject {
		Q_OBJECT

	public:
		HTTPWorker(QThread*, const QUrl&, const QUrlQuery&);
		~HTTPWorker();

		private Q_SLOTS:
		void post();

	private:
		QNetworkAccessManager manager;
		QUrlQuery params;
		QUrl url;

	Q_SIGNALS:
		void finished(QNetworkReply*);
	};

	class GCPConnector : public QObject {
		Q_OBJECT

	public:
		GCPConnector(QObject* parent = nullptr);

		/*
			returns capabilities and specifications for
			a printer(required in anonymous registeration)
		*/
		static QByteArray printerCapabilites(const QPrinter&);

		public Q_SLOTS:
		/* first time anonymous registration for connector-based Printers */
		void initRegister(const QPrinter& = QPrinter());

		/* complete printer registration & get authorization code */
		void getAuthorizationCode(const QUrl& polling_url, const QString& client_id);

		/* get access & refresh tokens as last step */
		void getTokens(const QString& client_id, const QString& secret, const QString& authorization_code);

	signals:
		void netError(NetworkError error_code, QString error_str);
		void apiError(int code, QString detail);
		void registered(QString token, QString printer_id, QUrl invite_url, QUrl polling_url, QTime time_out /* token life time */);
		void authorized(QString authorization_code, QString user_email, QUrl confirm_url);

	protected:
		void post(const QUrl&, const QUrlQuery&, std::function<void(QNetworkReply*)> /* slot */);

	private:
		struct APIData {
			QString access_token;
			QString refresh_token;
		} _data;
	};

api.cpp

#include "api.h"

namespace GCPAPI {

	HTTPWorker::HTTPWorker(QThread* thread, const QUrl& _url, const QUrlQuery& _params) :url{ _url }, params{ _params } {
		moveToThread(thread);
		manager.moveToThread(thread);

		QObject::connect(thread, &QThread::started, this, &HTTPWorker::post);
		QObject::connect(this, &HTTPWorker::finished, thread, &QThread::quit);
		QObject::connect(thread, &QThread::finished, thread, &QThread::deleteLater);
		QObject::connect(thread, &QThread::finished, this, &HTTPWorker::deleteLater);
	}

	HTTPWorker::~HTTPWorker() {

	}

	void HTTPWorker::post() {
		QNetworkRequest req(url);
		req.setHeader(QNetworkRequest::ContentTypeHeader, QStringLiteral("application/x-www-form-urlencoded"));
		const auto data = params.isEmpty() ? url.toEncoded() : params.toString(QUrl::FullyEncoded).toUtf8();
		manager.post(req, data);
		QObject::connect(&manager, &QNetworkAccessManager::finished, this, &HTTPWorker::finished);
	}


	GCPConnector::GCPConnector(QObject* parent) :QObject(parent) {
		
	}

	void GCPConnector::post(const QUrl& url, const QUrlQuery& params, std::function<void(QNetworkReply*)> slot) {
		QThread* local = new QThread;
		HTTPWorker* worker = new HTTPWorker(local, url, params);
		QObject::connect(worker, &HTTPWorker::finished, this, slot);
		local->start();
	}

	QByteArray GCPConnector::printerCapabilites(const QPrinter& printer) {
		QPrinterInfo info(printer);
		QJsonDocument doc;
		QJsonObject mainObj, printerObj;
		mainObj.insert(QStringLiteral("version"), QStringLiteral("1.0"));

		/* collate */
		{
			QJsonObject obj;
			obj.insert(QStringLiteral("default"), printer.collateCopies());
			printerObj.insert(QStringLiteral("collate"), obj);
		}

		/* content type */
		{
			QJsonObject obj;
			QJsonArray array;
			obj.insert(QStringLiteral("content_type"), QStringLiteral("application/pdf"));
			array << obj;
			printerObj.insert(QStringLiteral("supported_content_type"), array);
		}

		/* duplex mode */
		{
			QJsonObject obj;
			QJsonArray array;
			QJsonArray optionArray;
			const auto dmode = printer.duplex();
			QJsonObject obj1;
			obj1.insert(QStringLiteral("type"), QStringLiteral("NO_DUPLEX"));
			obj1.insert(QStringLiteral("is_default"), dmode == QPrinter::DuplexNone);
			QJsonObject obj2;
			obj2.insert(QStringLiteral("type"), QStringLiteral("LONG_EDGE"));
			obj2.insert(QStringLiteral("is_default"), dmode == QPrinter::DuplexLongSide);
			QJsonObject obj3;
			obj3.insert(QStringLiteral("type"), QStringLiteral("SHORT_EDGE"));
			obj3.insert(QStringLiteral("is_default"), dmode == QPrinter::DuplexShortSide);
			optionArray << obj1 << obj2 << obj3;
			obj.insert(QStringLiteral("option"), optionArray);

			printerObj.insert(QStringLiteral("duplex"), obj);
		}
		
		/* dpi */
		{
			QJsonArray array;
			QJsonObject optionObj;
			optionObj.insert(QStringLiteral("horizontal_dpi"), printer.physicalDpiX());
			optionObj.insert(QStringLiteral("vendor_id"), QString::number(printer.physicalDpiX()) + QStringLiteral("dpi"));
			optionObj.insert(QStringLiteral("vertical_dpi"), printer.physicalDpiY());
			optionObj.insert(QStringLiteral("custom_display_name"), QString::number(printer.physicalDpiY()) + QStringLiteral("dpi"));
			optionObj.insert(QStringLiteral("is_default"), true);
			array << optionObj;
			QJsonObject obj;
			obj.insert(QStringLiteral("option"), array);
			printerObj.insert(QStringLiteral("dpi"), obj);
		}

		/* color */
		{
			QJsonObject obj, obj1, obj2;
			QJsonArray optionArray;
			obj1.insert(QStringLiteral("type"), QStringLiteral("STANDARD_MONOCHROME"));
			obj1.insert(QStringLiteral("is_default"), printer.colorMode() == QPrinter::GrayScale);
			obj2.insert(QStringLiteral("type"), QStringLiteral("STANDARD_COLOR"));
			obj2.insert(QStringLiteral("is_default"), printer.colorMode() == QPrinter::Color);
			optionArray << obj1 << obj2;
			obj.insert(QStringLiteral("option"), optionArray);
			printerObj.insert(QStringLiteral("color"), obj);
		}
		
		/* vendor capabilities */
		{
			QJsonArray vendorArray;
			/* Page Orientation */
			QJsonObject poObj;
			poObj.insert(QStringLiteral("id"), QStringLiteral("PageOrientation"));
			poObj.insert(QStringLiteral("display_name"), QStringLiteral("PageOrientation"));
			poObj.insert(QStringLiteral("type"), QStringLiteral("SELECT"));
			poObj.insert(QStringLiteral("display_name"), QStringLiteral("PageOrientation"));
			QJsonArray tArray1;
			QJsonObject tObj1;
			{
				QJsonObject obj1;
				obj1.insert(QStringLiteral("display_name"), QStringLiteral("Portrait"));
				obj1.insert(QStringLiteral("is_default"), printer.orientation() == QPrinter::Portrait);
				obj1.insert(QStringLiteral("value"), QStringLiteral("Portrait"));
				QJsonObject obj2;
				obj2.insert(QStringLiteral("display_name"), QStringLiteral("Landscape"));
				obj2.insert(QStringLiteral("is_default"), printer.orientation() == QPrinter::Landscape);
				obj2.insert(QStringLiteral("value"), QStringLiteral("Landscape"));
				QJsonObject obj3;
				obj3.insert(QStringLiteral("display_name"), QStringLiteral("RotatedLandscape"));
				obj3.insert(QStringLiteral("is_default"), false);
				obj3.insert(QStringLiteral("value"), QStringLiteral("RotatedLandscape"));
				tArray1 << obj1 << obj2 << obj3;
			}
			tObj1.insert(QStringLiteral("option"), tArray1);
			poObj.insert(QStringLiteral("select_cap"), tObj1);

			/* Page Sizes */
			QJsonObject psObj;
			psObj.insert(QStringLiteral("id"), QStringLiteral("PageSize"));
			psObj.insert(QStringLiteral("display_name"), QStringLiteral("PageSize"));
			psObj.insert(QStringLiteral("type"), QStringLiteral("SELECT"));
			psObj.insert(QStringLiteral("display_name"), QStringLiteral("PageSize"));
			QJsonArray tArray2;
			QJsonObject tObj2;

			const auto psizes = info.supportedPageSizes();
			for (const auto psize : psizes) {
				QJsonObject t;
				t.insert(QStringLiteral("display_name"), psize.key());
				t.insert(QStringLiteral("value"), psize.name());
				if (tArray2.isEmpty())
					t.insert(QStringLiteral("is_default"), true);
				tArray2 << t;
			}
			tObj2.insert(QStringLiteral("option"), tArray2);
			psObj.insert(QStringLiteral("select_cap"), tObj2);
			
			vendorArray << poObj << psObj;
			printerObj.insert(QStringLiteral("vendor_capability"), vendorArray);
		}

		mainObj.insert(QStringLiteral("printer"), printerObj);
		doc.setObject(mainObj);

		return doc.toJson(QJsonDocument::Compact);
	}

	void GCPConnector::initRegister(const QPrinter& printer) {
		QUrl url(GCP_URL + GCP_REGISTER_PATH);
		QUrlQuery params;
		params.addQueryItem(QStringLiteral("name"), printer.printerName());
		params.addQueryItem(QStringLiteral("proxy"), QUuid::createUuid().toString());
		params.addQueryItem(QStringLiteral("capabilities"), printerCapabilites(printer));
		params.addQueryItem(QStringLiteral("use_cdd"), QStringLiteral("true"));
		log(QString("registering printer (%1) as anonymous to GCP...").arg(printer.printerName()));

		post(url, params, [this] (QNetworkReply* reply) {
			if (reply->error() != NetworkError::NoError) {
				log(QString("{%1(%2)} network error(%3)").arg(__FILE__).arg(__LINE__).arg(reply->errorString()));
				emit netError(reply->error(), reply->errorString());
				return;
			}

			const auto map = QJsonDocument::fromJson(reply->readAll()).object().toVariantMap();
			if (!map.find(QStringLiteral("success")).value().toBool()) {
				const auto msg = map.find(QStringLiteral("message")).value().toString();
				log(QString("{%1(%2)} API error(%3)").arg(__FILE__).arg(__LINE__).arg(msg));
				emit apiError(map.find(QStringLiteral("errorCode")).value().toInt(), msg);
				return;
			}

			const auto token = map.find(QStringLiteral("registration_token")).value().toString();
			const auto inviteURL = map.find(QStringLiteral("complete_invite_url")).value().toUrl();
			const auto pollingURL = map.find(QStringLiteral("polling_url")).value().toUrl();
			const auto timeOut = map.find(QStringLiteral("token_duration")).value().toInt();
			QString printerID;

			const auto items = QUrlQuery(pollingURL).queryItems();
			for (const auto id : items) {
				if (id.first == QStringLiteral("printerid"))
					printerID = id.second;
			}
			log(QString("anonymous registration successed (printerid:%1)").arg(printerID));
			emit registered(token, printerID, inviteURL, pollingURL, QTime(0, 0, 0).addSecs(timeOut));
		});
	}

	void GCPConnector::getAuthorizationCode(const QUrl& _url, const QString& client_id) {
		QUrl url;
		url.setUrl(_url.toString() + client_id);
		log("getting API authorization code...");

		post(url, QUrlQuery(), [this](QNetworkReply* reply) {
			if (reply->error()) {
				log(QString("{%1(%2)} network error(%3)").arg(__FILE__).arg(__LINE__).arg(reply->errorString()));
				emit netError(reply->error(), reply->errorString());
				return;
			}

			const auto map = QJsonDocument::fromJson(reply->readAll()).object().toVariantMap();
			if (!map.find(QStringLiteral("success")).value().toBool()) {
				const auto msg = map.find(QStringLiteral("message")).value().toString();
				log(QString("{%1(%2)} API error(%3)").arg(__FILE__).arg(__LINE__).arg(msg));
				emit apiError(0, msg);
				return;
			}
			
			const auto auth_code = map.find(QStringLiteral("authorization_code")).value().toString();
			const auto jabberID = map.find(QStringLiteral("xmpp_jid")).value().toString();
			const auto confirmURL = map.find(QStringLiteral("confirmation_page_url")).value().toUrl();
			const auto email = map.find(QStringLiteral("user_email")).value().toString();
			log(QString("printer has successfully registered for %1").arg(email));
			store(QStringLiteral("xmpp_jid"), jabberID);
			emit authorized(auth_code, email, confirmURL);
		});
	}

	void GCPConnector::getTokens(const QString& client_id, const QString& secret, const QString& authorization_code) {
		QUrl url(QStringLiteral("https://accounts.google.com/o/oauth2/token"));
		QUrlQuery params;
		params.addQueryItem(QStringLiteral("client_id"), client_id);
		params.addQueryItem(QStringLiteral("redirect_uri"), QStringLiteral("oob"));
		params.addQueryItem(QStringLiteral("client_secret"), secret);
		params.addQueryItem(QStringLiteral("grant_type"), QStringLiteral("authorization_code"));
		params.addQueryItem(QStringLiteral("code"), authorization_code);
		log("getting API access tokens...");

		post(url, params, [this](QNetworkReply* reply) {
			if (reply->error()) {
				const auto err_str = reply->errorString();
				log(QString("{%1(%2)} network error(%3)").arg(__FILE__).arg(__LINE__).arg(err_str));
				emit netError(reply->error(), err_str);
				return;
			}

			const auto map = QJsonDocument::fromJson(reply->readAll()).object().toVariantMap();
			const auto rtoken = map.find(QStringLiteral("refresh_token")).value().toString();
			const auto atoken = map.find(QStringLiteral("access_token")).value().toString();
			const auto expire_time = map.find(QStringLiteral("expires_in")).value().toInt();
			log("printer registration process done! tokens received...");
			store(QStringLiteral("refresh_token"), rtoken);
			QTimer::singleShot((expire_time - 60) * 1000, this, SLOT(onTokenExpire()));
			_data.access_token = atoken;
			_data.refresh_token = rtoken;
		});
	}

	void GCPConnector::onTokenExpire() {

	}

	void GCPConnector::apiCall() {
		QUrlQuery params;
		params.addQueryItem(QStringLiteral("access_token"), _data.access_token);
	}

} // namespace GCPAPI

don't know what part i'm missing!

[raven-worx]

@IMAN4K said in Why QNetworkReply run-time crash on deleteLater() ?:

someSlot is a lambda in initRegister function in my source code:

well that makes big difference and doesn't actually relate to the code you've posted.

The problem is:
QObject::connect(worker, &HTTPWorker::finished, this, slot)

The signal-slot connection remains "active" as long as worker and this is still alive.
So any previous lambda method is still called when the finished is triggered again as long as this is true. So a reply object from a previous call might be deleted, but is still executed.
You need to disconnect the signal-slot connection when all the work has been done or at least make sure.

Some possible solutions:

void GCPConnector::post(const QUrl& url, const QUrlQuery& params, std::function<void(QNetworkReply*)> slot) {
                 ...
		QObject::connect(worker, &HTTPWorker::finished, reply, slot);  // bind to reply somehow
                ...
	}

void GCPConnector::post(const QUrl& url, const QUrlQuery& params, std::function<void(QNetworkReply*)> slot) {
		QThread* local = new QThread;
		HTTPWorker* worker = new HTTPWorker(local, url, params);
		QObject::connect(worker, &HTTPWorker::finished, worker , slot); // bind to worker, and ensure it is deleted when it has finished work, etc.
		local->start();
	}

[IMAN4K]

@raven-worx

So any previous lambda method is still called when the finished is triggered again as long as this is true

Sorry for late reply.
I'm not sure but think this is not true because Although connection is alive until sender & receiver is alive BUT after thread destruction, sender OR the worker get deleted too! and there is no sender to trigger signal (am i right ?)
QObject::connect(thread, &QThread::finished, this, &HTTPWorker::deleteLater);

[IMAN4K]

At long last! i decided to simply process reply in worker thread(Thanks @raven-worx ):

        using Header = QMap<QString, QString>;

	class HttpWorker : public QObject {
		Q_OBJECT

	public:
		struct HttpData {
			bool hasError = false;
			NetworkError error = NetworkError::NoError;
			QString errorString;
			QByteArray data;
		};
		HttpWorker(QThread*, const QUrl&, const QUrlQuery&);
		~HttpWorker();

		void addHeaders(const Header&);

		enum Method {
			Post, Get
		};
		void setMethod(Method m) {
			http_method = m;
		}
		Method method() const {
			return http_method;
		}

		private Q_SLOTS:
		void sendRequest();

	private:
		QNetworkAccessManager manager;
		Header headers;
		QUrlQuery params;
		QUrl url;
		Method http_method = Post;

	private:
		void check(QNetworkReply*);
		void appendHeaders(QNetworkRequest*);

	Q_SIGNALS:
		void finished(HttpData);
	};

	HttpWorker::HttpWorker(QThread* thread, const QUrl& _url, const QUrlQuery& _params) :url{ _url }, params{ _params } {
		qRegisterMetaType<HttpData>("HttpData");
		moveToThread(thread);
		manager.moveToThread(thread);

		QObject::connect(thread, &QThread::started, this, &HttpWorker::sendRequest);
		QObject::connect(this, &HttpWorker::finished, thread, &QThread::quit);
		QObject::connect(thread, &QThread::finished, thread, &QThread::deleteLater);
		QObject::connect(thread, &QThread::finished, this, &HttpWorker::deleteLater);
		QObject::connect(&manager, &QNetworkAccessManager::finished, this, &HttpWorker::check);
	}

	HttpWorker::~HttpWorker() {

	}

	void HttpWorker::addHeaders(const Header& _headers) {
		headers = std::move(_headers);
	}

	void HttpWorker::appendHeaders(QNetworkRequest* req) {
		if (!req)
			return;

		QMapIterator<QString, QString> iterator(headers);
		while (iterator.hasNext()) {
			iterator.next();
			req->setRawHeader(QByteArray::fromStdString(iterator.key().toStdString()), QByteArray::fromStdString(iterator.value().toStdString()));
		}
	}

	void HttpWorker::sendRequest() {
		const auto content_h = QStringLiteral("application/x-www-form-urlencoded");
		if (method() == Post) {
			QNetworkRequest req(url);
			req.setHeader(QNetworkRequest::ContentTypeHeader, content_h);
			if (!headers.isEmpty())
				appendHeaders(&req);
			const auto data = params.isEmpty() ? url.toEncoded() : params.toString(QUrl::FullyEncoded).toUtf8();
			manager.post(req, data);
		} else {
			url.setQuery(params);
			QNetworkRequest req(url);
			req.setHeader(QNetworkRequest::ContentTypeHeader, content_h);
			manager.get(req);
		}
	}

	void HttpWorker::check(QNetworkReply* reply) {
		HttpData data;
		if (reply->error()) {
			data.hasError = true;
			data.error = reply->error();
			data.errorString = reply->errorString();
		} else {
			data.data = std::move(reply->readAll());
		}
		emit finished(std::move(data));

		reply->deleteLater();
	}