KDevelop on Mac leaves lots of `kdevelop*` and `preamble*` files behind in the user's temp. directory (`$TMPDIR`). That's probably something platform-specific in Qt but I've decided to look in to it. For instance, patch files aren't removed when quitting a session, at least not all of them. Looking at the code I notice a subtle difference in the open/close management: ``` void VCSDiffPatchSource::updateFromDiff(VcsDiff vcsdiff) { if(!m_file.isValid()) { QTemporaryFile temp2(QDir::tempPath() + QLatin1String("/kdevelop_XXXXXX.patch")); temp2.setAutoRemove(false); temp2.open(); QTextStream t2(&temp2); t2 << vcsdiff.diff(); qCDebug(VCS) << "filename:" << temp2.fileName(); m_file = QUrl::fromLocalFile(temp2.fileName()); temp2.close(); }else{ QFile file(m_file.path()); file.open(QIODevice::WriteOnly); QTextStream t2(&file); t2 << vcsdiff.diff(); } ``` When creating a new diff the QTemporaryFile instance is opened and closed explicitly while the QFile instance used to update the file contents is opened but never closed. I don't see however how this would change the effect of the explicit `QFile::remove(m_file...)` in the `VCSDiffPatchSource` dtor?! (BTW, shouldn't that remove instruction be conditional on m_file.isValid()?)
Erm, I just notice that the kdevelop_*.patch files are also left behind on Linux (in /tmp on my system)!
Ah, that's because ``` void PatchReviewPlugin::setPatch( IPatchSource* patch ) { //... if( m_patch ) { disconnect( m_patch.data(), &IPatchSource::patchChanged, this, &PatchReviewPlugin::notifyPatchChanged ); if ( qobject_cast<LocalPatchSource*>( m_patch ) ) { // make sure we don't leak this // TODO: what about other patch sources? m_patch->deleteLater(); ``` As far as I recall there's a fallback m_patch instance which should never be deleted. If that's still correct shouldn't this code deleteLater all m_patch instances except when they're of the type of the fallback? At the very least `VCSDiffPatchSource` instances should be deleted, I guess. Will test.
Created attachment 105074 [details] clean up VCSDiffPatchSource/PatchReview diff files Rough patch that does the job.
Temp. files that remain open after launching a session with 3 projects (KDevelop, KDevPlatform, Purpose) and 13 files opened for editing : $TMPDIR/preamble-d4511c.pch $TMPDIR/kdevelop.BM7249 $TMPDIR/kdevelop.Kj7249 $TMPDIR/kdevelop.qx7249 $TMPDIR/kdevelop.ux7249 $TMPDIR/kdevelop.vx7249 $TMPDIR/preamble-d4511c.pch $TMPDIR/kdevelop.LM7249 $TMPDIR/kdevelop.KM7249 $TMPDIR/kdevelop.EW7249 $TMPDIR/kdevelop.bD7249 $TMPDIR/kdevelop.Jj7249 $TMPDIR/kdevelop.On7249 $TMPDIR/kdevelop.MM7249 This time they were all cleaned up when I quit KDevelop, but that will of course not happen when the application terminates uncleanly. Can't these files be unlinked after creation so that they disappear when closed (and don't show up in the directory)?
Created attachment 105088 [details] clean up some of the parser temp files This reduces disk clutter related to clang-based parsing, apparently without side-effects (tested on Linux only for now). I'm not sure whether the explicit m_definesFile.close() has any benefit but it does seem like the sort of context in which subtle platform differences could exist.
An observation not directly related to the topic at hand: void PatchReviewPlugin::documentClosed(KDevelop::IDocument*) QUrl("file:///home/bertin/work/src/Scratch/KDE/KF5/kdevelop-git-5/languages/clang/duchain/parsesession.cpp") void PatchReviewPlugin::documentClosed(KDevelop::IDocument*) QUrl("file:///home/bertin/work/src/Scratch/KDE/KF5/kdevplatform-git-5/plugins/patchreview/patchreviewtoolview.cpp") void PatchReviewPlugin::documentClosed(KDevelop::IDocument*) QUrl("file:///home/bertin/work/src/Scratch/KDE/KF5/kdevplatform-git-5/plugins/patchreview/patchreviewtoolview.cpp") void PatchReviewPlugin::documentClosed(KDevelop::IDocument*) QUrl("file:///home/bertin/work/src/Scratch/KDE/KF5/kdevplatform-git-5/plugins/git/gitplugin.cpp") That's output after quitting KDevelop with a qDebug() call in PatchReviewPlugin::documentClosed(). I think the document controller signals should be disconnected from PatchReview slots when one returns from that plugin, no?
Comment on attachment 105074 [details] clean up VCSDiffPatchSource/PatchReview diff files diff --git plugins/patchreview/patchreview.cpp plugins/patchreview/patchreview.cpp index 06d514945bad42a4f7169bdd7f633db4fed9e859..f222bbb311bc9924592893c81e720a51636dbd63 100644 --- plugins/patchreview/patchreview.cpp +++ plugins/patchreview/patchreview.cpp @@ -58,12 +58,12 @@ #include <sublime/area.h> #include <sublime/document.h> #include <sublime/view.h> +#include <vcs/widgets/vcsdiffpatchsources.h> #include "patchhighlighter.h" #include "patchreviewtoolview.h" #include "localpatchsource.h" #include "debug.h" - Q_LOGGING_CATEGORY(PLUGIN_PATCHREVIEW, "kdevplatform.plugins.patchreview") using namespace KDevelop; @@ -489,10 +500,18 @@ void PatchReviewPlugin::setPatch( IPatchSource* patch ) { if( m_patch ) { disconnect( m_patch.data(), &IPatchSource::patchChanged, this, &PatchReviewPlugin::notifyPatchChanged ); - if ( qobject_cast<LocalPatchSource*>( m_patch ) ) { + if ( qobject_cast<LocalPatchSource*>( m_patch ) + || qobject_cast<VCSDiffPatchSource*>( m_patch ) ) { // make sure we don't leak this // TODO: what about other patch sources? + IDocument* patchDocument = ICore::self()->documentController()->documentForUrl( m_patch->file() ); + if (patchDocument) { + // it certainly shouldn't hurt to close the diff document now instead of at some later point. + patchDocument->close(IDocument::Discard); + } m_patch->deleteLater(); + } else { + qCWarning(PLUGIN_PATCHREVIEW) << "LEAKING" << m_patch << m_patch->name() << m_patch->file(); } } m_patch = patch;
Created attachment 105089 [details] clean up VCSDiffPatchSource/PatchReview diff files
Thank you for reporting this issue in KDE software. As it has been a while since this issue was reported, can we please ask you to see if you can reproduce the issue with a recent software version? If you can reproduce the issue, please change the status to "REPORTED" when replying. Thank you!
Dear Bug Submitter, This bug has been in NEEDSINFO status with no change for at least 15 days. Please provide the requested information as soon as possible and set the bug status as REPORTED. Due to regular bug tracker maintenance, if the bug is still in NEEDSINFO status with no change in 30 days the bug will be closed as RESOLVED > WORKSFORME due to lack of needed information. For more information about our bug triaging procedures please read the wiki located here: https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging If you have already provided the requested information, please mark the bug as REPORTED so that the KDE team knows that the bug is ready to be confirmed. Thank you for helping us make KDE software even better for everyone!
This bug has been in NEEDSINFO status with no change for at least 30 days. The bug is now closed as RESOLVED > WORKSFORME due to lack of needed information. For more information about our bug triaging procedures please read the wiki located here: https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging Thank you for helping us make KDE software even better for everyone!
Git commit a37c72dfd724a1bd2a587b46cc8a2ecf521d5001 by Igor Kushnir. Committed on 02/10/2023 at 19:50. Pushed by igorkushnir into branch 'master'. Clean up libclang's temporary files on KDevelop start I plan to place other temporary files created by KDevelop into the session temporary directory in the future. For example, KDevelop leaves behind many temporary kdevelop.* files after exit. Not all KDevelop tests remove their temporary files on exit. The session temporary directory use can be wrapped into convenient classes derived from QTemporaryDir and QTemporaryFile. Related: bug 412707 FIXED-IN: 5.13.231200 M +3 -0 kdevplatform/interfaces/icore.h M +75 -0 kdevplatform/shell/core.cpp M +1 -0 kdevplatform/shell/core.h M +2 -0 kdevplatform/shell/core_p.h M +47 -7 plugins/clang/duchain/clangindex.cpp https://invent.kde.org/kdevelop/kdevelop/-/commit/a37c72dfd724a1bd2a587b46cc8a2ecf521d5001