Bug 362485 - After refresh editor (F5) already set breakpoint(s) are disappearing
Summary: After refresh editor (F5) already set breakpoint(s) are disappearing
Status: RESOLVED FIXED
Alias: None
Product: kdevelop
Classification: Applications
Component: CPP Debugger (show other bugs)
Version: git master
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: kdevelop-bugs-null
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-30 00:54 UTC by Piotr Mierzwinski
Modified: 2024-03-08 10:23 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.14
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Mierzwinski 2016-04-30 00:54:09 UTC
Being in debug mode, and having set some breakpoint(s), invoking F5 (refresh editor) made that all breakpoints disappearing (from editor window and from Breakpoints view).

Reproducible: Always

Steps to Reproduce:
All breakpoints disappearing after refresh of editor window

Actual Results:  
The breakpoints should not disappear after refresh of editor window

Expected Results:  
My system is based on: Qt-5.6.0, Plasma-5.6.3, KDE Frameworks 5.21. Antergos (Arch based) distribution.
KDevelop and KDevplatform cloned from 5.0 branch at 26.04.2016
Comment 1 Piotr Mierzwinski 2016-04-30 01:09:01 UTC
Description of problem:
Being in debug mode, and having set some breakpoint(s), invoking F5 (refresh editor) made that all breakpoints disappearing (from editor window and from Breakpoints view).

Reproducible: Always

Steps to Reproduce:
1
2
3

Actual Results:  
All breakpoints disappearing after refresh of editor window

Expected Results:  
The breakpoints should not disappear after refresh of editor window

Additional Information:
My system is based on: Qt-5.6.0, Plasma-5.6.3, KDE Frameworks 5.21. Antergos (Arch based) distribution.
KDevelop and KDevplatform cloned from 5.0 branch at 26.04.2016
Comment 2 Piotr Mierzwinski 2016-05-16 22:40:14 UTC
I happens also in Code perspective.
Tested with KDevelop and KDevplatform cloned from 5.0 branch at 14.05.2016

Steps to Reproduce:
1. Open any cpp file
2. Set breakpoint
3. Refresh editor (press  F5)
Comment 3 Piotr Mierzwinski 2021-01-26 18:54:12 UTC
In the newest version I have, so coming from git master r263.gf8afa94aee (buit at Jan 11th) I observed that just disappear break point in place where application stopped. Breakpoint also disappear from view "Breakpoints".

I set two breakpoints in the same method. Application stopped at first one. After that I pressed F5. In result this breakpoint disappeared. Second one was still visible.

Used debugger is gdb.
Comment 4 JATothrim 2023-10-11 23:38:05 UTC
This bug should be confirmed.

Effects are:
- Document breakpoint markers vanish, but the breakpoint still appears in the list.
- Breakpoints may get deleted by document reload.
- Possible crash, if edited but not saved document is reload in debug session.
- Set breakpoint condition vanishes on reload.

Not caused by this bug:
- Breakpoint markers "jump" when saving document or toggling a breakpoint.

My analysis:

On document reload, KTextEditor component triggers BreakpointModel::markChanged() in kdevelop that eventually calls into BreakpointModel::updateMarks(). This cascades into even more markChanged() events during reload that only make the effect worse.

markChanged() removes and re-adds breakpoints.

All this happens on document reload. The breakpoints you see afterwards may not be the same instances, the original breakpoints are lost along with their condition and ignore hits.

The faulty, or rather entirely missing handling of reload is isolated to BreakpointModel. The used debugger backend should not matter.

I have nick named this bug as "marker cascade" :)
Comment 5 JATothrim 2024-02-27 20:52:51 UTC
A partial fix is now available in KDevelop master:
- The breakpoint view enabled, ignorehits, and condition are no longer lost if a document is reloaded.
- Reloading a saved document in KDevelop should work.
- Reloading and, if prompted with unsaved changes to the document, choosing "Discard" in the dialog works most of the time.
Comment 6 Igor Kushnir 2024-03-08 10:23:18 UTC
Git commit a1524591a3d0d02a493c3742d2c772e9f7b50654 by Igor Kushnir.
Committed on 08/03/2024 at 10:22.
Pushed by igorkushnir into branch 'master'.

BreakpointModel: complete document reload support

This commit addresses the following issue and removes the FIXME comment
that describes it: if a modified document is reloaded, and the user
clicks a button other than "Discard" in the message box that appears,
the moving cursors should not be reverted to their saved locations.

When a modified document is reloaded, the user makes a
Cancel/Discard/Save choice after the aboutToReload() signal is emitted.
Therefore, BreakpointModel::aboutToReload() must not make any changes to
breakpoints or breakpoint marks that depend on this user choice. Remove
moving cursors during a reload later - in
BreakpointModel::aboutToInvalidateMovingInterfaceContent() instead of
BreakpointModel::aboutToReload().

If the user cancels reloading a modified document, it does not emit the
signal aboutToInvalidateMovingInterfaceContent(), its moving cursors are
not invalidated and breakpoints remain intact.

If the user reloads a modified document and opts to save it (and
overwrite possible changes on disk), BreakpointModel::documentSaved() is
invoked and saves the breakpoints' tracked line numbers before
aboutToInvalidateMovingInterfaceContent() removes the moving cursors.

If the user reloads a modified document and opts to discard editor
changes or reloads an unmodified document, the executed BreakpointModel
code is almost unchanged, because these cases were already perfectly
supported. The only consequential difference is that aboutToReload()
inhibits mark change instead of removing all breakpoint marks in the
reloaded document. That's because the breakpoint marks must not be
removed in case the user cancels reloading.

Unfortunately, not removing breakpoint marks in aboutToReload() lets
KTextEditor::DocumentPrivate::documentReload() store in a local variable
all of the document's breakpoint marks. The saving and restoring of
breakpoint marks by KTextEditor is useless work, because BreakpointModel
has a different notion/algorithm where breakpoint marks should be placed
after the document is reloaded. There is no API to prevent this useless
KTextEditor work. So BreakpointModel resorts to working around possible
mark conflicts by removing all breakpoint marks in the document before
adding them at correct locations in case of the Discard user choice. In
case of the Save user choice or when an unmodified document is reloaded,
KTextEditor restores breakpoint marks at correct locations.
BreakpointModel still restores breakpoint marks again afterwards,
because KTextEditor may choose to skip restoring some of the marks. This
double restoration is safe, because KTextEditor::Document's member
function addMark(int line, uint markType) does not change anything if a
mark of the type `markType` is already set on the line `line`.

Connect to 3 KTextEditor::Document's signals - aboutToReload(),
aboutToInvalidateMovingInterfaceContent() and reloaded() in
BreakpointModel::textDocumentCreated(). The downside of connecting to
every open document's signals is that both the connecting itself and the
work in the slots when the document is reloaded is useless if there are
no breakpoints at its URL (which should be the case for most documents).

I have considered and rejected connecting to these 3 signals in
BreakpointModel::setupMovingCursor(), which is called only for documents
that contain breakpoints. The problem is that a moving cursor is not set
up for a breakpoint at a line out of the document's range. But reloading
may increase the number of lines in the document and thus allow
BreakpointModel::reloaded() to enable document line tracking for
newly-in-range breakpoints.

In the future, BreakpointModel should acquire a way to efficiently
access all breakpoints in a given document. This would substantially
reduce overhead for documents without breakpoints and allow to move the
existing aboutToDeleteMovingInterfaceContent() connection from
setupMovingCursor() to textDocumentCreated().

A unit test cannot make the Cancel/Discard/Save user choice
programmatically instead of showing the Close Document warning dialog.
Add semiautomatic tests and skip them by default. When a developer makes
code changes that affect breakpoints in a reloaded modified document,
[s]he can disable the QSKIP() call and run the tests.

Extract two helper functions from
TestBreakpointModel::testDocumentReload() and reuse them. The extracted
code is practically unchanged.
FIXED-IN: 5.14

M  +6    -0    kdevplatform/debugger/breakpoint/breakpoint.cpp
M  +7    -0    kdevplatform/debugger/breakpoint/breakpoint.h
M  +107  -31   kdevplatform/debugger/breakpoint/breakpointmodel.cpp
M  +4    -3    kdevplatform/debugger/breakpoint/breakpointmodel.h
M  +170  -27   kdevplatform/debugger/tests/test_breakpointmodel.cpp
M  +8    -0    kdevplatform/debugger/tests/test_breakpointmodel.h

https://invent.kde.org/kdevelop/kdevelop/-/commit/a1524591a3d0d02a493c3742d2c772e9f7b50654