Skip to content

Commit fa41062

Browse files
Ri0nRi0n
committed
ServerInfoManager: use self-deleting qobject isntead of callback to handle slot context automatically
this solves a crash on s5b proxy discovery and early jingle session close
1 parent e50f3d3 commit fa41062

File tree

4 files changed

+163
-147
lines changed

4 files changed

+163
-147
lines changed

‎src/xmpp/xmpp-im/httpfileupload.cpp

+60-61
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@
2121

2222
#include "xmpp_client.h"
2323
#include "xmpp_serverinfomanager.h"
24-
#include "xmpp_tasks.h"
2524
#include "xmpp_xmlcommon.h"
2625

2726
#include <QList>
2827
#include <QNetworkAccessManager>
2928
#include <QNetworkReply>
3029
#include <QNetworkRequest>
30+
#include <QPointer>
3131

3232
using namespace XMPP;
3333

@@ -99,70 +99,69 @@ void HttpFileUpload::start()
9999
if (featureOptions.isEmpty()) {
100100
featureOptions << (QSet<QString>() << xmlns_v0_2_5) << (QSet<QString>() << xmlns_v0_3_1);
101101
}
102-
d->client->serverInfoManager()->queryServiceInfo(
102+
auto query = d->client->serverInfoManager()->queryServiceInfo(
103103
QLatin1String("store"), QLatin1String("file"), featureOptions,
104-
QRegularExpression("^(upload|http|stor|file|dis|drive).*"), ServerInfoManager::SQ_CheckAllOnNoMatch,
105-
[this](const QList<DiscoItem> &items) {
106-
d->httpHosts.clear();
107-
for (const auto &item : items) {
108-
const QStringList &l = item.features().list();
109-
XEP0363::version ver = XEP0363::vUnknown;
110-
QString xmlns;
111-
quint64 sizeLimit = 0;
112-
if (l.contains(xmlns_v0_3_1)) {
113-
ver = XEP0363::v0_3_1;
114-
xmlns = xmlns_v0_3_1;
115-
} else if (l.contains(xmlns_v0_2_5)) {
116-
ver = XEP0363::v0_2_5;
117-
xmlns = xmlns_v0_2_5;
104+
QRegularExpression("^(upload|http|stor|file|dis|drive).*"), ServiceInfoQuery::CheckAllOnNoMatch);
105+
connect(query, &ServiceInfoQuery::finished, this, [this](const QList<DiscoItem> &items) {
106+
d->httpHosts.clear();
107+
for (const auto &item : items) {
108+
const QStringList &l = item.features().list();
109+
XEP0363::version ver = XEP0363::vUnknown;
110+
QString xmlns;
111+
quint64 sizeLimit = 0;
112+
if (l.contains(xmlns_v0_3_1)) {
113+
ver = XEP0363::v0_3_1;
114+
xmlns = xmlns_v0_3_1;
115+
} else if (l.contains(xmlns_v0_2_5)) {
116+
ver = XEP0363::v0_2_5;
117+
xmlns = xmlns_v0_2_5;
118+
}
119+
if (ver != XEP0363::vUnknown) {
120+
QVector<std::pair<HttpHost, int>> hosts;
121+
const XData::Field field = item.registeredExtension(xmlns).getField(QLatin1String("max-file-size"));
122+
if (field.isValid() && field.type() == XData::Field::Field_TextSingle)
123+
sizeLimit = field.value().at(0).toULongLong();
124+
HttpHost host;
125+
host.ver = ver;
126+
host.jid = item.jid();
127+
host.sizeLimit = sizeLimit;
128+
QVariant metaProps(d->client->serverInfoManager()->serviceMeta(host.jid, "httpprops"));
129+
if (metaProps.isValid()) {
130+
host.props = HostProps(metaProps.value<int>());
131+
} else {
132+
host.props = SecureGet | SecurePut;
133+
if (ver == XEP0363::v0_3_1)
134+
host.props |= NewestVer;
118135
}
119-
if (ver != XEP0363::vUnknown) {
120-
QVector<std::pair<HttpHost, int>> hosts;
121-
const XData::Field field = item.registeredExtension(xmlns).getField(QLatin1String("max-file-size"));
122-
if (field.isValid() && field.type() == XData::Field::Field_TextSingle)
123-
sizeLimit = field.value().at(0).toULongLong();
124-
HttpHost host;
125-
host.ver = ver;
126-
host.jid = item.jid();
127-
host.sizeLimit = sizeLimit;
128-
QVariant metaProps(d->client->serverInfoManager()->serviceMeta(host.jid, "httpprops"));
129-
if (metaProps.isValid()) {
130-
host.props = HostProps(metaProps.value<int>());
131-
} else {
132-
host.props = SecureGet | SecurePut;
133-
if (ver == XEP0363::v0_3_1)
134-
host.props |= NewestVer;
135-
}
136-
int value = 0;
137-
if (host.props & SecureGet)
138-
value += 5;
139-
if (host.props & SecurePut)
140-
value += 5;
141-
if (host.props & NewestVer)
142-
value += 3;
143-
if (host.props & Failure)
144-
value -= 15;
145-
if (!sizeLimit || d->fileSize < sizeLimit)
146-
hosts.append({ host, value });
147-
148-
// no sorting in preference order. most preferred go first
149-
std::sort(hosts.begin(), hosts.end(),
150-
[](const auto &a, const auto &b) { return a.second > b.second; });
151-
for (auto &hp : hosts) {
152-
d->httpHosts.append(hp.first);
153-
}
136+
int value = 0;
137+
if (host.props & SecureGet)
138+
value += 5;
139+
if (host.props & SecurePut)
140+
value += 5;
141+
if (host.props & NewestVer)
142+
value += 3;
143+
if (host.props & Failure)
144+
value -= 15;
145+
if (!sizeLimit || d->fileSize < sizeLimit)
146+
hosts.append({ host, value });
147+
148+
// no sorting in preference order. most preferred go first
149+
std::sort(hosts.begin(), hosts.end(), [](const auto &a, const auto &b) { return a.second > b.second; });
150+
for (auto &hp : hosts) {
151+
d->httpHosts.append(hp.first);
154152
}
155153
}
156-
// d->currentHost = d->httpHosts.begin();
157-
d->client->httpFileUploadManager()->setDiscoHosts(d->httpHosts);
158-
if (d->httpHosts.isEmpty()) { // if empty as the last resort check all services
159-
d->result.statusCode = HttpFileUpload::ErrorCode::NoUploadService;
160-
d->result.statusString = "No suitable http upload services were found";
161-
done(State::Error);
162-
} else {
163-
tryNextServer();
164-
}
165-
});
154+
}
155+
// d->currentHost = d->httpHosts.begin();
156+
d->client->httpFileUploadManager()->setDiscoHosts(d->httpHosts);
157+
if (d->httpHosts.isEmpty()) { // if empty as the last resort check all services
158+
d->result.statusCode = HttpFileUpload::ErrorCode::NoUploadService;
159+
d->result.statusString = "No suitable http upload services were found";
160+
done(State::Error);
161+
} else {
162+
tryNextServer();
163+
}
164+
});
166165
}
167166

168167
void HttpFileUpload::tryNextServer()

‎src/xmpp/xmpp-im/jingle-s5b.cpp

+34-34
Original file line numberDiff line numberDiff line change
@@ -698,42 +698,42 @@ namespace XMPP { namespace Jingle { namespace S5B {
698698

699699
proxyDiscoveryInProgress = true;
700700
QList<QSet<QString>> featureOptions = { { "http://jabber.org/protocol/bytestreams" } };
701-
q->_pad->session()->manager()->client()->serverInfoManager()->queryServiceInfo(
701+
auto query = q->_pad->session()->manager()->client()->serverInfoManager()->queryServiceInfo(
702702
QStringLiteral("proxy"), QStringLiteral("bytestreams"), featureOptions,
703-
QRegularExpression("proxy.*|socks.*|stream.*|s5b.*"), ServerInfoManager::SQ_CheckAllOnNoMatch,
704-
[this](const QList<DiscoItem> &items) {
705-
if (!proxyDiscoveryInProgress) { // check if new results are ever/still expected
706-
// seems like we have successful connection via higher priority channel. so nobody cares
707-
// about proxy
708-
return;
709-
}
710-
auto m = static_cast<Manager *>(q->_pad->manager());
711-
Jid userProxy = m->userProxy();
712-
713-
bool userProxyFound = !userProxy.isValid();
714-
for (const auto &i : items) {
715-
quint16 localPref = 0;
716-
if (!userProxyFound && i.jid() == userProxy) {
717-
localPref = 1;
718-
userProxyFound = true;
719-
continue;
720-
}
721-
Candidate c(q, i.jid(), generateCid(), localPref);
722-
localCandidates.emplace(c.cid(), c);
723-
qDebug("new local candidate: %s", qPrintable(c.toString()));
724-
queryS5BProxy(i.jid(), c.cid());
725-
}
726-
if (!userProxyFound) {
727-
Candidate c(q, userProxy, generateCid(), 1);
728-
localCandidates.emplace(c.cid(), c);
729-
qDebug("new local candidate: %s", qPrintable(c.toString()));
730-
queryS5BProxy(userProxy, c.cid());
731-
} else if (items.count() == 0) {
732-
// seems like we don't have any proxy
733-
proxyDiscoveryInProgress = false;
734-
checkAndFinishNegotiation();
703+
QRegularExpression("proxy.*|socks.*|stream.*|s5b.*"), ServiceInfoQuery::CheckAllOnNoMatch);
704+
q->connect(query, &ServiceInfoQuery::finished, q, [this](const QList<DiscoItem> &items) {
705+
if (!proxyDiscoveryInProgress) { // check if new results are ever/still expected
706+
// seems like we have successful connection via higher priority channel. so nobody cares
707+
// about proxy
708+
return;
709+
}
710+
auto m = static_cast<Manager *>(q->_pad->manager());
711+
Jid userProxy = m->userProxy();
712+
713+
bool userProxyFound = !userProxy.isValid();
714+
for (const auto &i : items) {
715+
quint16 localPref = 0;
716+
if (!userProxyFound && i.jid() == userProxy) {
717+
localPref = 1;
718+
userProxyFound = true;
719+
continue;
735720
}
736-
});
721+
Candidate c(q, i.jid(), generateCid(), localPref);
722+
localCandidates.emplace(c.cid(), c);
723+
qDebug("new local candidate: %s", qPrintable(c.toString()));
724+
queryS5BProxy(i.jid(), c.cid());
725+
}
726+
if (!userProxyFound) {
727+
Candidate c(q, userProxy, generateCid(), 1);
728+
localCandidates.emplace(c.cid(), c);
729+
qDebug("new local candidate: %s", qPrintable(c.toString()));
730+
queryS5BProxy(userProxy, c.cid());
731+
} else if (items.count() == 0) {
732+
// seems like we don't have any proxy
733+
proxyDiscoveryInProgress = false;
734+
checkAndFinishNegotiation();
735+
}
736+
});
737737
}
738738

739739
void tryConnectToRemoteCandidate()

‎src/xmpp/xmpp-im/xmpp_serverinfomanager.cpp

+29-18
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,14 @@ void ServerInfoManager::queryServicesList()
9494
jtitems->go(true);
9595
}
9696

97+
void ServerInfoManager::finish(ServiceInfoQuery *q, const QList<DiscoItem> &items)
98+
{
99+
emit q->finished(items);
100+
if (q->parent() == this) {
101+
q->deleteLater();
102+
}
103+
}
104+
97105
void ServerInfoManager::checkPendingServiceQueries()
98106
{
99107
// if services list is not ready yet we have to exit. if it's failed we have to finish all pending queries
@@ -102,17 +110,18 @@ void ServerInfoManager::checkPendingServiceQueries()
102110
const auto sqs = _serviceQueries;
103111
_serviceQueries.clear();
104112
for (const auto &q : sqs) {
105-
q.callback(QList<DiscoItem>());
113+
finish(q);
106114
}
107115
}
108116
return;
109117
}
110118

111119
// services list is ready here and we can start checking it and sending disco#info to not cached entries
112-
auto sqIt = _serviceQueries.begin();
113-
while (sqIt != _serviceQueries.end()) {
120+
auto queryIt = _serviceQueries.begin();
121+
while (queryIt != _serviceQueries.end()) {
114122

115123
// populate services to query for this service request
124+
auto sqIt = *queryIt;
116125
if (!sqIt->servicesToQueryDefined) {
117126
sqIt->spareServicesToQuery.clear();
118127
// grep all suitble service jids. moving forward preferred ones
@@ -122,7 +131,7 @@ void ServerInfoManager::checkPendingServiceQueries()
122131
if (sqIt->nameHint.isValid()) {
123132
if (!sqIt->nameHint.isValid() || sqIt->nameHint.match(si.key()).hasMatch()) {
124133
sqIt->servicesToQuery.push_back(si.key());
125-
} else if (sqIt->options & SQ_CheckAllOnNoMatch) {
134+
} else if (sqIt->options & ServiceInfoQuery::CheckAllOnNoMatch) {
126135
sqIt->spareServicesToQuery.push_back(si.key());
127136
}
128137
} else {
@@ -134,8 +143,8 @@ void ServerInfoManager::checkPendingServiceQueries()
134143
sqIt->spareServicesToQuery.clear();
135144
}
136145
if (sqIt->servicesToQuery.empty()) {
137-
sqIt->callback(QList<DiscoItem>());
138-
_serviceQueries.erase(sqIt++);
146+
finish(sqIt);
147+
_serviceQueries.erase(queryIt++);
139148
continue;
140149
}
141150
sqIt->servicesToQueryDefined = true;
@@ -169,7 +178,7 @@ void ServerInfoManager::checkPendingServiceQueries()
169178
sqIt->features.constBegin(), sqIt->features.constEnd(), false,
170179
[&si](bool a, const QSet<QString> &b) { return a || si->item.features().test(b); }))) {
171180
sqIt->result.append(si->item);
172-
if (sqIt->options & SQ_FinishOnFirstMatch) {
181+
if (sqIt->options & ServiceInfoQuery::FinishOnFirstMatch) {
173182
break;
174183
}
175184
}
@@ -210,20 +219,19 @@ void ServerInfoManager::checkPendingServiceQueries()
210219
}
211220

212221
// if has at least one sufficient result
213-
auto forceFinish = (!sqIt->result.isEmpty() && (sqIt->options & SQ_FinishOnFirstMatch)); // stop on first found
222+
auto forceFinish = (!sqIt->result.isEmpty()
223+
&& (sqIt->options & ServiceInfoQuery::FinishOnFirstMatch)); // stop on first found
214224
// if nothing in progress then we have full result set or nothing found even in spare list
215225
if (forceFinish || !hasInProgress) { // self explanatory
216-
auto callback = std::move(sqIt->callback);
217-
auto result = sqIt->result;
218-
_serviceQueries.erase(sqIt++);
219-
callback(result);
226+
_serviceQueries.erase(queryIt++);
227+
finish(sqIt, sqIt->result);
220228
} else {
221-
++sqIt;
229+
++queryIt;
222230
}
223231
}
224232
}
225233

226-
void ServerInfoManager::appendQuery(const ServiceQuery &q)
234+
void ServerInfoManager::appendQuery(ServiceInfoQuery *q)
227235
{
228236
_serviceQueries.push_back(q);
229237
if (_servicesListState == ST_InProgress) {
@@ -236,11 +244,14 @@ void ServerInfoManager::appendQuery(const ServiceQuery &q)
236244
}
237245
}
238246

239-
void ServerInfoManager::queryServiceInfo(const QString &category, const QString &type,
240-
const QList<QSet<QString>> &features, const QRegularExpression &nameHint,
241-
SQOptions options, std::function<void(const QList<DiscoItem> &items)> callback)
247+
ServiceInfoQuery *ServerInfoManager::queryServiceInfo(const QString &category, const QString &type,
248+
const QList<QSet<QString>> &features,
249+
const QRegularExpression &nameHint,
250+
ServiceInfoQuery::Options options)
242251
{
243-
appendQuery(ServiceQuery(type, category, features, nameHint, options, std::move(callback)));
252+
auto query = new ServiceInfoQuery(type, category, features, nameHint, options, this);
253+
appendQuery(query);
254+
return query;
244255
}
245256

246257
void ServerInfoManager::setServiceMeta(const Jid &service, const QString &key, const QVariant &value)

0 commit comments

Comments
 (0)