Avoid unnecessarily storing document in actions

[jonahgraham]

With the first part of the fix in #603 we recreate the document regularly (on each context change) therefore we need to let any consumers of the document fetch the current version of the document from the editor (by calling getAdapter(IDocument.class)) when it is needed, rather than caching it.

Fixes #603

[jonahgraham]

@betamaxbandit WDYT of this fix? I tried to track down other places that may hold onto the document after getting it from the viewer.

[betamaxbandit]

@betamaxbandit WDYT of this fix? I tried to track down other places that may hold onto the document after getting it from the viewer.

I've applied your changes to the Disassembly view and my fork of it and I can no longer reproduce the org.eclipse.jface.text.BadLocationException exception I was experiencing when inserting breakpoints.
I'll try more testing tomorrow.
Thanks for this!

[betamaxbandit]

@betamaxbandit WDYT of this fix? I tried to track down other places that may hold onto the document after getting it from the viewer.

I've applied your changes to the Disassembly view and my fork of it and I can no longer reproduce the org.eclipse.jface.text.BadLocationException exception I was experiencing when inserting breakpoints. I'll try more testing tomorrow. Thanks for this!

I've done more testing this morning on the Disassembly view. Good news, I was unable to reproduce the original bug.
These are the tests I performed:

1. Go to current program counter (Home)
2. Go to address...
3. Refresh
4. Show source
5. Open new view
6. Toggle breakpoint; double click, right click
7. Find in current document
8. Switching back and forth between different active debug contexts
9. Navigating up and down through memory
10. Single stepping instructions
11. Pin to debug context
12. Jump to memory; opens Memory view

@jonahgraham can you think of any other testing or functionality I have not tested?

[jonahgraham]

Thanks @betamaxbandit for the testing. I did one additional test. I added the following code to make sure there was nothing holding onto to the "old" document:

From 7295aa7c79640961c6415f8153869c0c99191751 Mon Sep 17 00:00:00 2001
From: Jonah Graham <jonah@kichwacoders.com>
Date: Thu, 2 Nov 2023 12:13:22 -0400
Subject: [PATCH] Some logging to make sure that Document is not referenced

---
 .../ui/disassembly/model/DisassemblyDocument.java     | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java
index 400aa18839..1e7e9ff322 100644
--- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java
+++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java
@@ -25,6 +25,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.StringJoiner;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.eclipse.cdt.debug.internal.ui.disassembly.dsf.AddressRangePosition;
 import org.eclipse.cdt.debug.internal.ui.disassembly.dsf.DisassemblyPosition;
@@ -86,9 +87,19 @@ public class DisassemblyDocument extends REDDocument implements IDisassemblyDocu
 	private double fMeanSizeOfInstructions = 4;
 
 	private long fErrorAlignment = 0x1L;
+	private int i;
+
+	private static AtomicInteger ai = new AtomicInteger(0);
 
 	public DisassemblyDocument() {
 		super();
+		i = ai.incrementAndGet();
+		System.out.println("Construct Document " + i);
+	}
+
+	@Override
+	protected void finalize() throws Throwable {
+		System.out.println("Finalize Document " + i);
 	}
 
 	/*
-- 
2.41.0

By running the garbage collector after changing contexts I can check that there are no handles to the "old" document.

Without this PR I can see this:

Construct Document 12
Construct Document 13
Finalize Document 11

where Document 13 is the one that is active, but there is also a Document 12 hanging around.

With this PR applied I can see that there are no other instances of DisassemblyDocument around as the log looks like this:

Construct Document 15
Construct Document 16
Finalize Document 14
Finalize Document 15

leaving Document 16 as the only instance of DisassemblyDocument (once I run GC).

To manually run GC, turn on heap status:

and the press the GC button in the status bar:

[jonahgraham]

I can't merge because of https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/3921

[jonahgraham]

@Kummallinen

[betamaxbandit]

Thanks @betamaxbandit for the testing. I did one additional test. I added the following code to make sure there was nothing holding onto to the "old" document:

Oh that's neat!
I've done a similar thing with my forked disassembly view and I see old document being released.