<table><tr><td style="">rjvbb added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D22424">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22424#inline-126610">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kossebau</span> wrote in <span style="color: #4b4d51; font-weight: bold;">textdocument.cpp:253</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Originall I wanted to use delete here only, and added the deleteLater as (temporary) work-around for the one known and any potentially 3rd-party action which has a Qt::QueuedConnection connect. The latter I see as questionable thing, as with context menus there is no promise t will exist any longer once the menu is closed, and the actions here are added to the context menu also passing ownership to it. Something I plan to fix afterwards, but did not want to mess with additionally, and also not change behaviour in the 5.3 branch.</p>

<p style="padding: 0; margin: 8px;">In general I also personally disfavour using deleteLater, as it makes it harder to track as human the life-time  management of objects and the order in which they are deleted, making debugging more complicated.</p>

<p style="padding: 0; margin: 8px;">A deleteLater also runs into troubles on shutdown, if all document objects & other stuff is deleted, where there might be no more event loop to reach afterwards. Think especially unit tests. Which is why in case of TextDocument destructor already now I propose to use delete.</p>

<p style="padding: 0; margin: 8px;">But given current unit tests are now running into issues with the need for extra eventloop hit for clean shutdown, and runtime also showed no issues, changed now to just always deleteLater. Private note made to revisit this once some patch to fix the SwitchToBuddy actions is done and accepted :)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Yes, <tt style="background: #ebebeb; font-size: 13px;">deleteLater()</tt> introduces a kind of object management as found in ObjC and Cocoa (refcounting and autorelease pools). What's missing in C++ is something that allows an object to cancel its own deleting when it detects that it is still in use (to avoid issues with deletion order).</p>

<p style="padding: 0; margin: 8px;">I haven't checked the code, but if deleteLater() causes the target object to be deleted when the current eventloop exits I would expect that this also happens when the last eventloop exits (and gets deleted). It's less clear what happens if you call the method after the main eventloop stopped:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">If deleteLater() is called after the main event loop has stopped, the object will not be deleted.
Since Qt 4.8, if deleteLater() is called on an object that lives in a thread with no running event loop, the object will be destroyed when the thread finishes.</pre></div>

<p style="padding: 0; margin: 8px;">You'd say that those 2 statements contradict each other.</p>

<p style="padding: 0; margin: 8px;">I did find something quite clever in the QSingleShotTimer ctor: <tt style="background: #ebebeb; font-size: 13px;">connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this, &QObject::deleteLater);</tt>.</p>

<p style="padding: 0; margin: 8px;">Anyway, I have run into multiple issues with window (and menu) objects being deleted directly on Mac, because of pending events which then got delivered to a stale (C++ or private ObjC) object. Hence the official Qt advice to use deleteLater() on UI element instances. Here it might be safe not to since the object(s) in question shouldn't be displayed at this point (I think?).</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22424#inline-126885">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kossebau</span> wrote in <span style="color: #4b4d51; font-weight: bold;">textdocument.cpp:771</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Indeed, qCWarning is better in here as long-term solution, where the Q_ASSERT only served me to quickly get kicked while drafting the code.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Thanks for confirming what I already expected about the presence of certain of those statements :)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R32 KDevelop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D22424">https://phabricator.kde.org/D22424</a></div></div><br /><div><strong>To: </strong>kossebau, KDevelop<br /><strong>Cc: </strong>anthonyfieroni, rjvbb, kdevelop-devel, hmitonneau, christiant, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd<br /></div>