[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: Race Condition Issue with KJob and QNetworkAccessManager
From:       Albert Astals Cid <aacid () kde ! org>
Date:       2013-03-07 14:31:31
Message-ID: 4904856.YlA2YRjdXl () xps
[Download RAW message or body]

El Dimarts, 5 de març de 2013, a les 21:25:13, Adrián Chaves Fernández va 
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:
> > 
> > 1. I think the access to "m_request" in FakeServer::*Request() needs
> > 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 some
> > point.
> > 
> > 2. The class FakeServer follows the "You're doing it wrong" pattern
> > (it contains the line "moveToThread(this);") , see
> > 
> > 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
> > 
> > This might or might not be part of the problem.
> 
> Many thanks for the input!
> 
> I think I've solve both issues, but the race condition persists.
> 
> I've uploaded a tar.xz with the current state of the test files to
> Gitorious:
> https://gitorious.org/libmediawiki/libmediawiki/blobs/raw/gallaecio/example
> s/raceissue/2ndAttemptToAvoidRaceCondition.tar.xz

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

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

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

Hope this helps.

Cheers,
  Albert

> 
> The files can also be found at
> https://gitorious.org/libmediawiki/libmediawiki/trees/gallaecio/examples/ra
> ceissue
> 
> I followed the lead of the Qt documentation, and the main function looks
> like this now:
> 
> int main(int argc, char* argv[])
> {
>     QCoreApplication a(argc, argv);
> 
>     QThread *serverThread = new QThread();
>     FakeServer *fakeserver = new FakeServer;
>     QObject::connect(serverThread, SIGNAL(started()), fakeserver,
> SLOT(run())); QObject::connect(serverThread, SIGNAL(finished()),
> fakeserver, SLOT(deleteLater())); fakeserver->moveToThread(serverThread);
>     serverThread->start();
> 
>     for (int i = 0; i < 10000; i++) {
> 
>         Job job;
>         job.exec();
> 
>         QList<FakeServer::Request> requests = fakeserver->getRequest();
>         if (requests.size() != 1) qDebug() << requests.size() << "? WTF?";
>         else                      qDebug() << requests.size() << ", as
> expected.";
> 
>         fakeserver->clearRequest();
>     }
> 
>     serverThread->quit();
> 
>     return 0;
> }
["proof1.tar.bz2" (proof1.tar.bz2)]

BZh91AY&SY`{?Ps+w1 ВHM252JyOM! 4
P
JBe2iLM4ѵ@4$D(`z L!h44424 

L@h$I4i4zhxe?P#@2he?`*= HP `hJy
4&&4M6kpEX"n,LD2Ild0RfɵhBh5~a=YR0f:+Ise&xhltgtklQ+3iCع\QTKs	!^. \
f-U[?Tح<}rO`E*jW_'٪r<훟 j60% \
i/ɦU:2,ݝ!<dТ,ѶyYiZSqj  \
.@9L3]S-"Z4S:[DN[ G3󥭞6갻Gjc/Xq/5 \
 }P Ԭ 
I0 ⿱LEnXJ0,ޮCVB.%@EHDJt"
f}QiAʑw[9sclV@ФQ4P)Jhiԏ۫ٱg[0}C9DDNٴ^%uZijZmʵV \
+Cs0rY:$ԛ|xKAi- j6۽U8OQNcEfh#H\PHǔ獴Y1d \
ոۍJԹ4okR](=F[zE١qS5zl]IMqmΟ*̚m(%WK#ss^?|[m"
 wl$o5
AjKl̑&0]:*
.[ -(YFY.B&ȓY50^0 WE9SgBxx \
Ɔc΄؊4M$E \
T8$QD;BFDVeoUG3(B2AruC02tՑ>jkpͽ.1,`Rpi3/* \
E[į@*EW97MA|!pHY`^8r13A`B \
q Xi 1H
g77
͙`2I
C+0LqY˻byB/P ;99] ڼ, DYlp"
@/X0X412J-sjQ$&a7B(aUV 6 T]WO] \
WXS#9#=pdFs8aX= F1eB:۳?NKcKt0{  UZpӲH
`


["proof2.tar.bz2" (proof2.tar.bz2)]

BZh91AY&SYUZD{?Pt
\hsLhij~)4S
4 Pz(E='Mѡ ɑ i H'zM4`!# \
`d2hѣM@M2@$d4h4ShS<S \
=F$?*^_KN	I3zBj$4И M4$/Ws41+J}IC)2 \
1A(+M!	 βL \
X]9t?<8YmsA^uM=mI3͖)uU-!ܞ;썶h L])l>T	 \
OTyM~r)R2L}nӚyƩ '5M$xb4#%sPy"ٚ=%'TwYCkFEL \
(? Bhr d@*^kqAc"$)QQ-rMjI[!#c_
zcJSX>jKκ_;9$;Bwݻ+/N;LpT+Ah]-ڰIܭhMe0bưz
 .?:spHC>(DSLm]6  ;bTKEމ
/h'ͻ水jmYhoeQӻ9zL<twAŇ9iXA`@YxʓADSaTb' \
0i|mZaCr5)=BIB+̂p #`w \
P>6:f>=%ǖd<ɮjA2-wz>|ǤnQ$yq;	f-IlcտPWrg`	' \
ZBcIHX+*]M	J.2*c	i661&*) \
вEC(ƕ@7jFYFkQchH[&3.\.+d/#0BCÀ`6^g \
&CߋrļDu%\ܗNQF`m{y	^Tj)=b.ݪ{C6V>fFm \
`Eh<x!:Ykt7P_.ud2[KnkPqV"zCzHlIʀtkb]";%@ڴGʡ2NpH9L \
COQ1m 2-]5 ݔv
Ɖ	&1QE%3
Ph5,f9c
͖?aʑEI)h *2eA 1-u룦шXZ+3n`ӄ&"(H{"



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


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic