----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127437/#review93863 -----------------------------------------------------------
Ship it! Ship It! - David Edmundson On March 20, 2016, 2:46 p.m., David Faure wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127437/ > ----------------------------------------------------------- > > (Updated March 20, 2016, 2:46 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kio > > > Description > ------- > > * Major race with other threads due to using QDir::setCurrent() > * Race conditions on m_terminationRequested and m_matches > * Race with previous completion thread when its posted event arrives after > cancelling > * Cancellation code spread out in many methods and never done fully correctly > * isRunning() was missing one of the two threads, making unit test fail in > valgrind > * Fix the rarely-hit code path where the thread isn't finished after 200ms > - the current search string was lost because finished() wasn't called > - the matches were not used, in case of user-completion (AFAICS) > - changing the search string while the thread was running could lead to > the old search > string still being used for completion > (the misnamed finished() wasn't called, so KCompletion didn't get the > new string) > => added a variant of the unittest which doesn't wait for the thread > initially > > + Simplify code using signal/slots rather than a custom event > + Simplify code using enum rather than casting to/from int > > Most of these bugs are from 2004 (e.g. commit ec83c408619e3) ;) > > REVIEW: 127437 > > > Diffs > ----- > > autotests/CMakeLists.txt 4dff0a24d31897a9641a70017bd87e33415cef14 > autotests/kurlcompletiontest.cpp eef39ff17940fadcb9492e5e7092070c976310d4 > src/widgets/CMakeLists.txt 87dac50b377f6ea4c3cd39e9afa37a4680aecf31 > src/widgets/kurlcompletion.h 14fda22a0e08bcf2da30e19fed577c8bda64a4ca > src/widgets/kurlcompletion.cpp 1dc8f4898fb78d6f49e687462446007a10305f98 > > Diff: https://git.reviewboard.kde.org/r/127437/diff/ > > > Testing > ------- > > Hit a crash in DirectoryListThread when playing with kopenwithtest. > > kurlcompletiontest extended, kurlcompletion-nowait added, both pass, also in > valgrind (different timings). > > > Thanks, > > David Faure > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
