> On June 16, 2016, 7:57 a.m., David Faure wrote: > > tests/kjobtrackerstest.cpp, line 173 > > <https://git.reviewboard.kde.org/r/128113/diff/4/?file=468220#file468220line173> > > > > Is this change supposed to trigger the bug? I added those two connects > > locally (without the rest of the patch) and kjobtrackerstest still works > > fine. > > > > Is there a way to see the bug without ark? > > > > The ark code mixes KJob with QThread, which KJob wasn't meant for. It's > > an interesting experiment, but I have my doubts as to whether it's done > > correctly... > > Anthony Fieroni wrote: > Ark calls in this order > terminate > unregister > terminate > unregister > Yes, make it 2 times > > Anthony Fieroni wrote: > Or KJob' pointer is danglig when try to disconnect it cause a crash. > > Elvis Angelaccio wrote: > > The ark code mixes KJob with QThread, which KJob wasn't meant for. It's > an interesting experiment, but I have my doubts as to whether it's done > correctly... > > I've tried in the past the get rid of that QThread, but with little > success. How else can you handle a long-lasting KJob without freezing the UI? > > David Faure wrote: > Most KJobs are async operations (e.g. network downloads), so such > long-lasting KJobs in the main thread are fine, there's plenty of opportunity > to go back to the event loop and refresh the GUI. > If this is mostly a computational job then indeed a separate thread (or > process) is needed. It just shouldn't mess with the KJob, i.e. the KJob must > be "one layer on top of the thread", and fully running in the main thread. > > Anthony: which terminate method are you talking about? > And I did explain why the KJob pointer is dangling (deleted by secondary > thread due to wrong direct connection). > > Remove that direct connection, debug bug 193908 again, use helgrind, then > we can come back to this bug if it's still there. > > Anthony Fieroni wrote: > I mean: > finished > unregister > finished > unregister > So bug is fixed in the master, i offer partial fix only for ark 16.04 > branch > Here for me must be: > if (!d->progressJobView.contains(job)) > return; > KJobTrackerInterface::unregisterJob(job);
That's still a workaround. There's no reason for finished to be emitted twice for a given job. If this is indeed what's happening, please debug why it happens. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128113/#review96558 ----------------------------------------------------------- On June 10, 2016, 5:07 a.m., Anthony Fieroni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128113/ > ----------------------------------------------------------- > > (Updated June 10, 2016, 5:07 a.m.) > > > Review request for KDE Frameworks and David Faure. > > > Repository: kjobwidgets > > > Description > ------- > > + Fix memory leak in finished > > > Diffs > ----- > > src/kuiserverjobtracker.cpp ebed3a5 > tests/kjobtrackerstest.cpp 7ef9e07 > > Diff: https://git.reviewboard.kde.org/r/128113/diff/ > > > Testing > ------- > > ark --changetofirstpath --add --autofilename zip kjobwidgets > Crash before, fix with patch > > > File Attachments > ---------------- > > backtrace > > https://git.reviewboard.kde.org/media/uploaded/files/2016/06/06/5b87387e-7e7b-4982-b91b-a18f72414509__backtrace > memcheck > > https://git.reviewboard.kde.org/media/uploaded/files/2016/06/07/ba4b0150-5e01-4cc1-8776-7530d053d6f0__memcheck > memcheck 7 errorrs > > https://git.reviewboard.kde.org/media/uploaded/files/2016/06/07/f8813ccc-8835-4255-842c-29c57c2dea23__memcheck2 > > > Thanks, > > Anthony Fieroni > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
