> On Nov. 17, 2014, 2:45 p.m., Thomas Lübking wrote: > > attica-kde/kdeplugin/kdeplatformdependent.cpp, line 56 > > <https://git.reviewboard.kde.org/r/121161/diff/2/?file=328928#file328928line56> > > > > is > > > > KdePlatformDependent::~KdePlatformDependent() > > { > > if (QCoreApplication::closingDown()) > > m_accessManager->setParent(nullptr); > > } > > > > sufficient? > > Albert Astals Cid wrote: > Isn't that a more complex way of doing the same? > > Thomas Lübking wrote: > Depends on whether KdePlatformDependent is ever deleted in a running > application (where the leak would really matter) > If not, then yes: superfluous complication. > > Jeremy Whiting wrote: > Did I hear a "Ship it!" in there somewhere? > > Thomas Lübking wrote: > Leaving aside that don't maintain the attica plugin, you have a "+1" from > here, IF > a) the conditional reparent on destruction is not sufficient (doesn't > work) > OR > b) KdePlatformDependent object(s) are not supposed to be deleted during > the runtime of the application > > Ie. if the leak is inevitable or only theoretical.
Yep, I'm one of the attica and knewstuff maintainers and got the plugin to load recently, then hit this crash on close, so trying to fix it. The code in the plugin is only built with the plugin, so shouldn't be used besides as a plugin. The conditional reparent probably works too, but as Albert said it's just a more complicated way of doing the same thing. - Jeremy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121161/#review70542 ----------------------------------------------------------- On Nov. 17, 2014, 2:11 p.m., Jeremy Whiting wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121161/ > ----------------------------------------------------------- > > (Updated Nov. 17, 2014, 2:11 p.m.) > > > Review request for kdelibs and Plasma. > > > Repository: plasma-desktop > > > Description > ------- > > Plugin unload order was making the attica_kde plugin crash on close, > this works around it by leaking one AccessManager. > > > Diffs > ----- > > attica-kde/kdeplugin/kdeplatformdependent.cpp > 489c03b18b7bb940007ab51cb197105fbc25de9f > > Diff: https://git.reviewboard.kde.org/r/121161/diff/ > > > Testing > ------- > > knewstuff tests no longer crash on close. > > > Thanks, > > Jeremy Whiting > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
