Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-29 Thread Alberto Jiménez Ruiz
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/ --- (Updated Oct. 29, 2015, 9:19 p.m.) Status -- This change has been

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-28 Thread Alberto Jiménez Ruiz
> On Oct. 26, 2015, 5:27 p.m., Aleix Pol Gonzalez wrote: > > This patch introduced a crash here. Could you maybe look into it? Here's a > > backtrace. > > > > It happens on some (not all) programs when the open dialog is displayed. I > > can reproduce it with ksnapshot and kdevelop, but not

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-28 Thread Thomas Lübking
> On Okt. 26, 2015, 5:27 nachm., Aleix Pol Gonzalez wrote: > > This patch introduced a crash here. Could you maybe look into it? Here's a > > backtrace. > > > > It happens on some (not all) programs when the open dialog is displayed. I > > can reproduce it with ksnapshot and kdevelop, but not

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-28 Thread David Faure
> On Oct. 26, 2015, 5:27 p.m., Aleix Pol Gonzalez wrote: > > This patch introduced a crash here. Could you maybe look into it? Here's a > > backtrace. > > > > It happens on some (not all) programs when the open dialog is displayed. I > > can reproduce it with ksnapshot and kdevelop, but not

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-28 Thread Thomas Lübking
> On Okt. 26, 2015, 5:27 nachm., Aleix Pol Gonzalez wrote: > > This patch introduced a crash here. Could you maybe look into it? Here's a > > backtrace. > > > > It happens on some (not all) programs when the open dialog is displayed. I > > can reproduce it with ksnapshot and kdevelop, but not

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-28 Thread David Faure
> On Oct. 26, 2015, 5:27 p.m., Aleix Pol Gonzalez wrote: > > This patch introduced a crash here. Could you maybe look into it? Here's a > > backtrace. > > > > It happens on some (not all) programs when the open dialog is displayed. I > > can reproduce it with ksnapshot and kdevelop, but not

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-26 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review87445 --- This patch introduced a crash here. Could you maybe look into

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-24 Thread Alberto Jiménez Ruiz
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/ --- (Updated Oct. 24, 2015, 5:54 p.m.) Status -- This change has been

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-22 Thread Alberto Jiménez Ruiz
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/ --- (Updated Oct. 22, 2015, 7:31 a.m.) Review request for kdelibs, Albert

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-22 Thread Alberto Jiménez Ruiz
> On Oct. 22, 2015, 7:11 a.m., David Faure wrote: > > The fix looks fine to me now, thanks. The unittest can be improved a bit, > > feel free to commit after making the changes suggested below (or tell me if > > you have no account) I don't have an account to commit the changes. Improved the

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-22 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review87242 --- Ship it! The fix looks fine to me now, thanks. The unittest

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-22 Thread Albert Astals Cid
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review87278 --- I was about to commit this but i guess this also needs to be

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-19 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review87048 --- Thanks for the investigation and patch. kio/kio/copyjob.cpp

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-19 Thread David Faure
> On Oct. 19, 2015, 7:28 a.m., David Faure wrote: > > kio/kio/job.cpp, line 2671 > > > > > > Same question here. As the comment says, "the result is still OK", > > which is why I didn't set an error code here.

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-19 Thread Alberto Jiménez Ruiz
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/ --- (Updated Oct. 19, 2015, 9:43 a.m.) Review request for kdelibs, Albert

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-19 Thread Alberto Jiménez Ruiz
> On Oct. 19, 2015, 7:28 a.m., David Faure wrote: > > kio/kio/job.cpp, line 2671 > > > > > > Same question here. As the comment says, "the result is still OK", > > which is why I didn't set an error code here.

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-19 Thread Alberto Jiménez Ruiz
> On Oct. 19, 2015, 7:28 a.m., David Faure wrote: > > kio/kio/job.cpp, line 2671 > > > > > > Same question here. As the comment says, "the result is still OK", > > which is why I didn't set an error code here.

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-15 Thread Alberto Jiménez Ruiz
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/ --- (Updated Oct. 15, 2015, 7:33 a.m.) Review request for kdelibs and Albert

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-15 Thread Alberto Jiménez Ruiz
> On Oct. 14, 2015, 10:02 p.m., Albert Astals Cid wrote: > > Is it possible that your test is conflicting with others, without your > > patch all passes here but with it i get JobTest::directorySize and > > JobTest::listRecursive to fail Yeah, I forgot to delete the folders that the test had

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-14 Thread Albert Astals Cid
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review86866 --- Is it possible that your test is conflicting with others,

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-13 Thread Alberto Jiménez Ruiz
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/ --- (Updated Oct. 13, 2015, 10:48 a.m.) Review request for kdelibs. Bugs:

Review Request 125613: Race condition and error notification loss in ListJob

2015-10-12 Thread Alberto Jiménez Ruiz
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/ --- Review request for kdelibs. Repository: kdelibs Description ---

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-12 Thread Alberto Jiménez Ruiz
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/ --- (Updated Oct. 12, 2015, 7:11 p.m.) Review request for kdelibs. Bugs:

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-12 Thread Alberto Jiménez Ruiz
> On Oct. 12, 2015, 7:29 p.m., Albert Astals Cid wrote: > > Do you think you can add an unittest for this or is it hard to recreate the > > conditionts? It happens to very few people. Most developers tried to reproduce bug 333436 and bug 162211(I believe they are closely related) with no

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-12 Thread Thomas Lübking
> On Okt. 12, 2015, 7:29 nachm., Albert Astals Cid wrote: > > Do you think you can add an unittest for this or is it hard to recreate the > > conditionts? > > Alberto Jiménez Ruiz wrote: > It happens to very few people. Most developers tried to reproduce bug > 333436 and bug 162211(I

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-12 Thread Alberto Jiménez Ruiz
> On Oct. 12, 2015, 7:29 p.m., Albert Astals Cid wrote: > > Do you think you can add an unittest for this or is it hard to recreate the > > conditionts? > > Alberto Jiménez Ruiz wrote: > It happens to very few people. Most developers tried to reproduce bug > 333436 and bug 162211(I

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-12 Thread Albert Astals Cid
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review86738 --- Do you think you can add an unittest for this or is it hard

Re: Review Request 125613: Race condition and error notification loss in ListJob

2015-10-12 Thread Albert Astals Cid
> On oct. 12, 2015, 7:29 p.m., Albert Astals Cid wrote: > > Do you think you can add an unittest for this or is it hard to recreate the > > conditionts? > > Alberto Jiménez Ruiz wrote: > It happens to very few people. Most developers tried to reproduce bug > 333436 and bug 162211(I