From kde-devel  Thu Mar 07 14:31:31 2013
From: Albert Astals Cid <aacid () kde ! org>
Date: Thu, 07 Mar 2013 14:31:31 +0000
To: kde-devel
Subject: Re: Race Condition Issue with KJob and QNetworkAccessManager
Message-Id: <4904856.YlA2YRjdXl () xps>
X-MARC-Message: https://marc.info/?l=kde-devel&m=136266672827472
MIME-Version: 1
Content-Type: multipart/mixed; boundary="--nextPart3217014.aG7I0e10O5"

This is a multi-part message in MIME format.

--nextPart3217014.aG7I0e10O5
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"

El Dimarts, 5 de mar=C3=A7 de 2013, a les 21:25:13, Adri=C3=A1n Chaves =
Fern=C3=A1ndez va=20
escriure:
> O Martes, 5 de Marzo de 2013 11:53:13 Frank Reininghaus escribiu:
> > I'm not a network expert, and I don't know what the cause of the
> > problem is, but I noticed two things:
> >=20
> > 1. I think the access to "m_request" in FakeServer::*Request() need=
s
> > to be protected by locking m_mutex. Doing that does not seem to fix=

> > the issue, but not protecting it might lead to other problems at so=
me
> > point.
> >=20
> > 2. The class FakeServer follows the "You're doing it wrong" pattern=

> > (it contains the line "moveToThread(this);") , see
> >=20
> > http://blog.qt.digia.com/blog/2010/06/17/youre-doing-it-wrong/
> > http://woboq.com/blog/qthread-you-were-not-doing-so-wrong.html
> >=20
> > This might or might not be part of the problem.
>=20
> Many thanks for the input!
>=20
> I think I=E2=80=99ve solve both issues, but the race condition persis=
ts.
>=20
> I=E2=80=99ve uploaded a tar.xz with the current state of the test fil=
es to
> Gitorious:
> https://gitorious.org/libmediawiki/libmediawiki/blobs/raw/gallaecio/e=
xample
> s/raceissue/2ndAttemptToAvoidRaceCondition.tar.xz

There's no race condition at all between KJob and QNetworkAccessManager=
, i'd=20
say it's just that your code is assuming wrong stuff regarding how the =
network=20
communication happens.

Proof that there is no KJob/QNetworkAccessManager race condition is att=
ached=20
in proof1.tar.bz2. As you can see in it, there is no KJob anywhere but =
you=20
still get "Already got this request!!!!!!!!!!!" debugs

Proof that there is something wrong in your assumptions on how the netw=
ork=20
communication happens is attached in proof2.tar.bz2. As you see there I=
'm not=20
sending any data in the post method and all is fine. (You can do the sa=
me in=20
your code by sending nothing in your post() method inside job.cpp)

Hope this helps.

Cheers,
  Albert

>=20
> The files can also be found at
> https://gitorious.org/libmediawiki/libmediawiki/trees/gallaecio/examp=
les/ra
> ceissue
>=20
> I followed the lead of the Qt documentation, and the main function lo=
oks
> like this now:
>=20
> int main(int argc, char* argv[])
> {
>     QCoreApplication a(argc, argv);
>=20
>     QThread *serverThread =3D new QThread();
>     FakeServer *fakeserver =3D new FakeServer;
>     QObject::connect(serverThread, SIGNAL(started()), fakeserver,
> SLOT(run())); QObject::connect(serverThread, SIGNAL(finished()),
> fakeserver, SLOT(deleteLater())); fakeserver->moveToThread(serverThre=
ad);
>     serverThread->start();
>=20
>     for (int i =3D 0; i < 10000; i++) {
>=20
>         Job job;
>         job.exec();
>=20
>         QList<FakeServer::Request> requests =3D fakeserver->getReques=
t();
>         if (requests.size() !=3D 1) qDebug() << requests.size() << "?=
 WTF?";
>         else                      qDebug() << requests.size() << ", a=
s
> expected.";
>=20
>         fakeserver->clearRequest();
>     }
>=20
>     serverThread->quit();
>=20
>     return 0;
> }
--nextPart3217014.aG7I0e10O5
Content-Disposition: attachment; filename="proof1.tar.bz2"
Content-Transfer-Encoding: base64
Content-Type: application/x-bzip-compressed-tar; name="proof1.tar.bz2"

QlpoOTFBWSZTWehgzPMAA9l/lN30BgB7f//fP///nv///+sEAAEAAAhQBf5zHBordzGaoKadzdCS
SE0yNTKexNNKeU/VHqekzE0htR6gNAaADRkPUPUNqNBKnoRCZTJptUyNHqNNNNPU0bUDQAaANAaA
AAAkRCin6mCIeoGg2kwhkAABkAAAABkDgaaaaDQ0NDI0AyANDQGmjIAAGExAaCRJBMQ0aRo00npo
EnieqfqmZT9QI0AaMhiD1GgDZT9gKuv1Pf7dC48gSKWnUISkwOGgomAPaEp5DTQmJgA0iAITTQPL
C/42a+8aiZpwRf0bWLOarboiC24sEkyQRDIZSZBskY2NmmTAmMYwjVJm6oXJtWhCaPw1fsBhPVnA
Ug8wZgM6nfEAKwJJc2UmeMvAaJ+Q5WwOdNPoZ3RrbPpRFBQrM2m7Q/rYuQPtmlxR29VUpktztQkO
IYXXXi7CyNlmki1VW6w/ka++Be2nVNitPH3vq3IY65LFG09gowcTRSqaaldfBrvqJ40U84+z2apy
5TyV7ZufDWo2vs32BBYwmOUf0SUgxQTy32mOL8mmVToM7TIsnN2dITzSZOzQopXBESzRtnmwWWla
U5itcdRqDfLCEISlHbYuQDmZBNHF/kwHM81dBLanU5fVDi0MgiJaFzTIU8CdsjpbRMzU5KROW5XC
RzPzpa2e8uw26rC7R6fN3uv7pmq/Yxj2Lxfd1lhxtC8DmDXlCs0KfRDdBBv+3f+cAVAg1Ky4IAwK
STAg64Dt4r+xTEUBgo5uvwaplFgBEhtK3TCuLN6uQ4pWqUIukq4lhpXR7UBFSKSaREqhmnSdCyKS
Cmbe2cPzfRxRq7zR5GlBAsqRmIZ3GKK8mVs59ZRzmmNsVkDQpPpRlNg0UClKaJGx1GkZ1I/V9Nur
pvjZsWdbEdgwoX1DOURETtm0XqWN74cldaFaaWqQWokG+22vyrVWDsIr96ZDc/UH+Lcw7nLaWYQ6
46wki9SbfHi4i0sD9wxBAsGPaRItDbtqNtu9qFU4T9re1QZREU5jiYbcRWZoI0hcrVDhrBZIx5Tn
jbTBgNNZMalkmcLVuBXbjfFK1LkOizRva1KcXSg9+8xGss9b+RqwehJFkOvZoYBxU6kLiQSB0AiI
NYl6gO+YFKH6hGxdSeMPj01xbc6f9irMmm0oJVdLixCvI/xzc/Lx4V4/k6h841sIwG2QAePSIvHG
CnfIbCRv5jWNDUFqS2y0DMyRJjBdOioK4i5boK4t/ucox1mwCxiJGcYYikaFWS5CFrC1goad6hEm
yJNZpjUwXgGf6u0wAwaK+YqgV0W5OatTZ0J4eIC2oMaGEoyFGRVjzoTYijSbTYjl9hb9gMbbBiTF
RSBUHIs4iSRRRDtCj5xGRKoBu1aY9u0cms1lGm9VR4YWMwOAgygaQvyO6tsyH0Fy5hF11RjfEZUc
ohoWQxcGMNIyAXTVkRQ+qai+asYFa5tw6bfNvYEu0TEfDgEs4MBgpFLlqclwBOylFGkIMy8qoEW6
61ukxK9AKkVX8ZaTOQOG9zdN2hX82szMzEHEyX+qsObPfCGm8o/u2XBIFPpZGhBgXhuIwziK0HLE
BeITAITqxd3hMTNBrBb53xFgEBwI0EKCyqAOcRLhDCBYaYmOCjG+rkgK6vKXyBWBZzfhNwrNmWCN
2zIRiIIQ7kkNiUMQHCswTAahFHFZy7vHYvB5DopCGd8vUIzC0hmVojs51DldINq8gx4sEyBE4Fmf
kccDjOps6HAi+g1AL7JYzQy2lDCpFlg0mQQxkDLdSq4t7HMMahX2UecWGiQmtpJhEx43QoiFtihh
VVaQ8CDBNhbCVLxdBBCurYqaV9LawRHYT4UM7l2XoFfvs/oPWFONIzm8C9QjoT2ljp71rnDL6WTL
RnM4YVg9siBG+jGiZbrBletCOtuzP05Ly2NLdO8wew3sIFVacJjE07IBf+LuSKcKEh0MGZ5g

--nextPart3217014.aG7I0e10O5
Content-Disposition: attachment; filename="proof2.tar.bz2"
Content-Transfer-Encoding: base64
Content-Type: application/x-bzip-compressed-tar; name="proof2.tar.bz2"

QlpoOTFBWSZTWfdVWkQAA9Z/kt30AgB7f//fP///nv///+sEAAAAgAhQBf50zg0TXMYaaBXdc4SS
k0yYieJoaaJqfpMp6TT9U8oNqeoaADQAyaD1D1B6mQSmkygTRT0ntU2o0aGgyZGgAGmmgADQAAGg
SIkUJ6J6TTRgIaMRkxGjACPQCMJgCBpkDhoaMmjRo00MjIYQBkAMg00AADIGQBIkgmSp/po0hqZo
NNNTaKeZU8U8UyAaAAA9Ro0bJPU/kCru/F5fHeWe4JFLTqEJSYGajRwzgHpCaiQ00JiYANIgCE00
DyQvA69XtnMTNOCLMSusrc1K1REFfcrmSZIIhkMpMg2SMbGzQSgTGMYRpJSrnCtNqiEJo9TV6gCw
rA3AhhcBzrJMIAVYkl38pjkct3QdP6n7PLEY9jhZzA9t+nNBXnUV3U09bYxJkdkzzZYpdeO4VS3m
nSHB99yesTvsjbZoINdMjw7U2F0CjccpbLHWPvhUlozWCaCFrsRPDp5Uq3lNfhByKZ1SMqpMfelu
kbDTms55l+PGqerU1MKNBa4nNU3hJMG+gniYYjQjDNolc1D0eSLZmj0SFSUnVHeFFVkEQ2tGyee6
3EWJTIsVCijOP88gQhJo23L0BQ1kEo4P9kChsyr4G15roYVxQWPMIhsd7yQpuVHzUS1yTWq5SRLd
WyEjY/W/4+zgX5MWkgr3evVj0/pKU69Yxj5qS866FYJfO4g5JIU7QsXSG4CDd9279IArBBnsLwgC
5MhOCDtMFH8eE5CQcJoblM9UlStBDGhdwAstrNqwDEmjG9yt2milTWWwMPCItRRixrB6rKySlcgK
nM8urj+S6eCOOvdzcMEC2kjOQz4WKJ9ElqxTyR7jTG2KqYiZqV028aDCwaU7EaO2Ea2jv2KNVPZL
FEW63om6DeQvqGgngIHNu4X6GM/w5rC0qNpqbQtZaG+2ZflR07vOOQ6ZkNz+6HrWTBk8tpfwB3R3
BJFBs8WHOWlYf0FgQKxZtIkV5s147bbtscqThPkbtUGURFPa4mG3EVTNBGKLJyAwFc1pFnynvG2m
DAaau7KlpOBa+oFhuR0UzUOIcg60NbYpjj3q/vzsQqq69/IVEelJFkIrzIJwkSCIIwfK+wS9YHfM
ClD9PjY6ZoQ+PSXHlmTvtTzJrmpBMt8treF3Ec5/ej58uJeTx6RuqMtRFhskAHmzCLxxAp3oOwkb
u2axoagtSWyWgbbJEmMF1b9QVxFy8gK4t/2coxVnYBQYCQzfGImi6dknIIWkWkJjipmIkahJrPMc
7BdIzvXiWAwaK+QqgV0W4+StTZUJ4M4Fs4MaGEouyjIqY7EJsRRpNpsRlai8jAY22DEmKikCoNCy
iJJFFEO0KPQIxpVAN2rNHp2DRrNZRmvVUdsLEWOAg2gMwe1I41smM+YuXIIutwErZC8jMIZCmJKZ
QxjDgLhgmQ82l16yZ5QK1yZDqt+LcsS8RMR12CWUGAwUily53JcATspRRmCDbXvqgRbrrXkJiV6A
VIqv72qTKQOG/D2HsmJ/8ooZnZgDLhvdqsz4ntjTe0P1NlYSBT5mRvoLF23EYJxFaDyxAXiEwCE6
sPHwmFm+awW6dAi+BAcCN8hQX6oA5REuEMAF87amdWTRMrrAW0vQbhOr4tRrpNJQ+ZbtceHKViJ6
Q3pIbEnGEBvKgHQM4h7JFmv5Yl2uIqfBxjslh5FAsYbatEf9yqHK6QbF5Biw2TIETgX8uhxwSIk5
4kygEYVD1AYWtoGPTxFRkTFto7OghzKGjdEtp12ZhjUK3ZR2CsaJCaq5JhExvOhRELZFDCWlMwr2
C+lQ3OZorLnHHa6aiTUsZsMbvvE5GGMVu9UKzZY/YcoPypGEwAuQRUmuKWjij6Aq9TJlp0GgMS3f
daQI66Omi4uBgqrRiN/guOvzWFqZK+czB9huYAKq04TGJp34Bf+LuSKcKEh7qq0iAA==

--nextPart3217014.aG7I0e10O5
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

--nextPart3217014.aG7I0e10O5--