Skip to content

Commit 5ca68f6

Browse files
handle cases where multiple mappings of the same phsyical file shadow
eachother fixes the following case: file /foo/bar.cfc mappings foo.bar and whatever.foo.bar multiple mappings means there are 2 classfiles for one physical file breakpoint requests for foo/bar.cfc arrive sets up jvm breakpoints for one of the classfiles, but not the others we want to set breakpoints in all related classfiles. presumably their line mappings will all be the same. the same breakpointIDs (client visible) should be used for the same line in the same physical file, across multiple classfiles (so line 2 gets ID x in foo.bar.class and whatever.foo.bar.class)
1 parent 2aa8e8e commit 5ca68f6

File tree

5 files changed

+118
-24
lines changed

5 files changed

+118
-24
lines changed

luceedebug/src/main/java/luceedebug/Agent.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ private static Map<String, Integer> linearizedCoreInjectClasses() {
173173
result.put("luceedebug.coreinject.LuceeVm", 0);
174174
result.put("luceedebug.coreinject.ValTracker$CleanerRunner", 0);
175175
result.put("luceedebug.coreinject.ExprEvaluator", 0);
176+
177+
result.put("luceedebug.coreinject.Iife", 0);
178+
result.put("luceedebug.coreinject.Iife$Supplier2", 0);
176179

177180
result.put("luceedebug.coreinject.ExprEvaluator$Evaluator", 0);
178181
result.put("luceedebug.coreinject.ExprEvaluator$Lucee6Evaluator", 1);

luceedebug/src/main/java/luceedebug/DapServer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,8 +390,8 @@ public CompletableFuture<SetBreakpointsResponse> setBreakpoints(SetBreakpointsAr
390390
}
391391

392392
var result = new ArrayList<Breakpoint>();
393-
for (var cfBreakpoint : luceeVm_.bindBreakpoints(path, lines, exprs)) {
394-
result.add(map_cfBreakpoint_to_lsp4jBreakpoint(cfBreakpoint));
393+
for (IBreakpoint bp : luceeVm_.bindBreakpoints(path, lines, exprs)) {
394+
result.add(map_cfBreakpoint_to_lsp4jBreakpoint(bp));
395395
}
396396

397397
var response = new SetBreakpointsResponse();

luceedebug/src/main/java/luceedebug/OriginalAndTransformedString.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,13 @@ public OriginalAndTransformedString(String original, String transformed) {
77
this.original = original;
88
this.transformed = transformed;
99
}
10+
11+
@Override
12+
public boolean equals(Object v) {
13+
if (v instanceof OriginalAndTransformedString) {
14+
var other = (OriginalAndTransformedString)v;
15+
return other.original.equals(original) && other.transformed.equals(transformed);
16+
}
17+
return false;
18+
}
1019
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package luceedebug.coreinject;
2+
3+
class Iife {
4+
@FunctionalInterface
5+
public interface Supplier2<T> {
6+
T get() throws Throwable;
7+
}
8+
9+
@SuppressWarnings("unchecked")
10+
static public <T extends Throwable, R> R rethrowUnchecked(Object e) throws T {
11+
throw (T) e;
12+
}
13+
14+
static public <T> T iife(Supplier2<T> f) {
15+
try {
16+
return f.get();
17+
}
18+
catch (Throwable e) {
19+
return rethrowUnchecked(e);
20+
}
21+
}
22+
}

luceedebug/src/main/java/luceedebug/coreinject/LuceeVm.java

Lines changed: 82 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
import java.lang.ref.Cleaner;
44
import java.lang.ref.WeakReference;
5+
import java.util.Collection;
56
import java.util.ArrayList;
67
import java.util.Arrays;
78
import java.util.HashMap;
89
import java.util.HashSet;
910
import java.util.List;
11+
import java.util.Set;
1012
import java.util.concurrent.CompletableFuture;
1113
import java.util.concurrent.ConcurrentHashMap;
1214
import java.util.concurrent.atomic.AtomicBoolean;
@@ -24,6 +26,8 @@
2426
import java.util.concurrent.ExecutorService;
2527
import java.util.concurrent.Executors;
2628

29+
import static luceedebug.coreinject.Iife.iife;
30+
2731
import luceedebug.*;
2832

2933
public class LuceeVm implements ILuceeVm {
@@ -122,6 +126,20 @@ private static class ReplayableCfBreakpointRequest {
122126
* and ask it if it itself is bound? Does `isEnabled` yield that, or is that just "we asked for it to be enabled"?
123127
*/
124128
final BreakpointRequest maybeNull_jdwpBreakpointRequest;
129+
130+
@Override
131+
public boolean equals(Object vv) {
132+
if (!(vv instanceof ReplayableCfBreakpointRequest)) {
133+
return false;
134+
}
135+
var v = (ReplayableCfBreakpointRequest)vv;
136+
137+
return ideAbsPath.equals(v.ideAbsPath)
138+
&& serverAbsPath.equals(v.serverAbsPath)
139+
&& line == v.line
140+
&& id == v.id
141+
&& expr.equals(v.expr);
142+
}
125143

126144
ReplayableCfBreakpointRequest(String ideAbsPath, String serverAbsPath, int line, int id, String expr) {
127145
this.ideAbsPath = ideAbsPath;
@@ -141,15 +159,15 @@ private static class ReplayableCfBreakpointRequest {
141159
this.maybeNull_jdwpBreakpointRequest = jdwpBreakpointRequest;
142160
}
143161

144-
static List<BreakpointRequest> getJdwpRequests(ArrayList<ReplayableCfBreakpointRequest> vs) {
162+
static List<BreakpointRequest> getJdwpRequests(Collection<ReplayableCfBreakpointRequest> vs) {
145163
return vs
146164
.stream()
147165
.filter(v -> v.maybeNull_jdwpBreakpointRequest != null)
148166
.map(v -> v.maybeNull_jdwpBreakpointRequest)
149167
.collect(Collectors.toList());
150168
}
151169

152-
static BpLineAndId[] getLineInfo(ArrayList<ReplayableCfBreakpointRequest> vs) {
170+
static BpLineAndId[] getLineInfo(Collection<ReplayableCfBreakpointRequest> vs) {
153171
return vs
154172
.stream()
155173
.map(v -> new BpLineAndId(v.ideAbsPath, v.serverAbsPath, v.line, v.id, v.expr))
@@ -159,8 +177,16 @@ static BpLineAndId[] getLineInfo(ArrayList<ReplayableCfBreakpointRequest> vs) {
159177

160178
private final ThreadMap threadMap_ = new ThreadMap();
161179
private final ExecutorService stepHandlerExecutor = Executors.newSingleThreadExecutor();
162-
private final ConcurrentHashMap</*canonical sourceAbsPath*/ String, ArrayList<ReplayableCfBreakpointRequest>> replayableBreakpointRequestsByAbsPath_ = new ConcurrentHashMap<>();
163-
private final ConcurrentHashMap</*canonical absPath*/ String, KlassMap> klassMap_ = new ConcurrentHashMap<>();
180+
private final ConcurrentHashMap</*canonical sourceAbsPath*/ String, Set<ReplayableCfBreakpointRequest>> replayableBreakpointRequestsByAbsPath_ = new ConcurrentHashMap<>();
181+
182+
/**
183+
* Mapping of "abspath on disk" -> "class file info"
184+
* Where a single path on disk can map to zero-or-more associated class files.
185+
* Usually there is only 1 classfile per abspath, but runtime mappings can mean that a single file
186+
* like "/app/foo.cfc" maps to "myapp.foo" as well as "someOtherMapping.foo", where each mapping
187+
* is represented by a separate classfile.
188+
*/
189+
private final ConcurrentHashMap</*canonical absPath*/ String, Set<KlassMap>> klassMap_ = new ConcurrentHashMap<>();
164190
private long JDWP_WORKER_CLASS_ID = 0;
165191
private ThreadReference JDWP_WORKER_THREADREF = null;
166192

@@ -570,8 +596,12 @@ private void trackClassRef(ReferenceType refType) {
570596

571597
final var klassMap = maybeNull_klassMap; // definitely non-null
572598

573-
var replayableBreakpointRequests = replayableBreakpointRequestsByAbsPath_.get(klassMap.sourceName.transformed);
574-
klassMap_.put(klassMap.sourceName.transformed, klassMap);
599+
Set<ReplayableCfBreakpointRequest> replayableBreakpointRequests = replayableBreakpointRequestsByAbsPath_.get(klassMap.sourceName.transformed);
600+
601+
klassMap_
602+
.computeIfAbsent(klassMap.sourceName.transformed, _z -> new HashSet<>())
603+
.add(klassMap);
604+
575605
if (replayableBreakpointRequests != null) {
576606
rebindBreakpoints(klassMap.sourceName.transformed, replayableBreakpointRequests);
577607
}
@@ -737,16 +767,22 @@ static KlassMap maybeNull_tryBuildKlassMap(Config config, ReferenceType refType)
737767
// unreachable
738768
return null;
739769
}
770+
771+
@Override
772+
public boolean equals(Object e) {
773+
if (e instanceof KlassMap) {
774+
return ((KlassMap)e).sourceName.equals(this.sourceName);
775+
}
776+
return false;
777+
}
740778
}
741779

742780
private AtomicInteger breakpointID = new AtomicInteger();
743781
private int nextBreakpointID() {
744782
return breakpointID.incrementAndGet();
745783
}
746784

747-
public void rebindBreakpoints(String serverAbsPath, ArrayList<ReplayableCfBreakpointRequest> cfBpRequests) {
748-
System.out.println("Rebinding breakpoints for " + serverAbsPath);
749-
785+
public void rebindBreakpoints(String serverAbsPath, Collection<ReplayableCfBreakpointRequest> cfBpRequests) {
750786
var changedBreakpoints = __internal__bindBreakpoints(serverAbsPath, ReplayableCfBreakpointRequest.getLineInfo(cfBpRequests));
751787

752788
if (breakpointsChangedCallback != null) {
@@ -777,8 +813,25 @@ private BpLineAndId[] freshBpLineAndIdRecordsFromLines(OriginalAndTransformedStr
777813
}
778814

779815
var result = new BpLineAndId[lines.length];
816+
817+
Set<ReplayableCfBreakpointRequest> bpInfo = replayableBreakpointRequestsByAbsPath_.get(absPath.transformed);
818+
780819
for (var i = 0; i < lines.length; ++i) {
781-
result[i] = new BpLineAndId(absPath.original, absPath.transformed, lines[i], nextBreakpointID(), exprs[i]);
820+
final int line = lines[i];
821+
822+
int id = iife(() -> {
823+
if (bpInfo == null) {
824+
return nextBreakpointID();
825+
}
826+
for (var z : bpInfo) {
827+
if (z.line == line) {
828+
return z.id;
829+
}
830+
}
831+
return nextBreakpointID();
832+
});
833+
834+
result[i] = new BpLineAndId(absPath.original, absPath.transformed, line, id, exprs[i]);
782835
}
783836
return result;
784837
}
@@ -792,12 +845,12 @@ public IBreakpoint[] bindBreakpoints(OriginalAndTransformedString absPath, int[]
792845
* i.e. the IDE might say "/foo/bar/baz.cfc" but we are only aware of "/app-host-container/foo/bar/baz.cfc" or etc.
793846
*/
794847
private IBreakpoint[] __internal__bindBreakpoints(String serverAbsPath, BpLineAndId[] lineInfo) {
795-
final var klassMap = klassMap_.get(serverAbsPath);
848+
final Set<KlassMap> klassMapSet = klassMap_.get(serverAbsPath);
796849

797-
if (klassMap == null) {
798-
var replayable = new ArrayList<ReplayableCfBreakpointRequest>();
850+
if (klassMapSet == null) {
851+
var replayable = replayableBreakpointRequestsByAbsPath_.computeIfAbsent(serverAbsPath, _z -> new HashSet<>());
799852

800-
var result = new Breakpoint[lineInfo.length];
853+
IBreakpoint[] result = new Breakpoint[lineInfo.length];
801854
for (int i = 0; i < lineInfo.length; i++) {
802855
final var ideAbsPath = lineInfo[i].ideAbsPath;
803856
final var shadow_serverAbsPath = lineInfo[i].serverAbsPath; // should be same as first arg to this method, kind of redundant
@@ -809,12 +862,19 @@ private IBreakpoint[] __internal__bindBreakpoints(String serverAbsPath, BpLineAn
809862
replayable.add(new ReplayableCfBreakpointRequest(ideAbsPath, shadow_serverAbsPath, line, id, expr));
810863
}
811864

812-
replayableBreakpointRequestsByAbsPath_.put(serverAbsPath, replayable);
813-
814865
return result;
815866
}
816867

817-
return __internal__idempotentBindBreakpoints(klassMap, lineInfo);
868+
IBreakpoint[] bpListPerMapping = new IBreakpoint[0];
869+
870+
clearExistingBreakpoints(serverAbsPath);
871+
872+
for (KlassMap mapping : klassMapSet) {
873+
bpListPerMapping = __internal__idempotentBindBreakpoints(mapping, lineInfo);
874+
}
875+
876+
// return just the last one
877+
return bpListPerMapping;
818878
}
819879

820880

@@ -823,9 +883,7 @@ private IBreakpoint[] __internal__bindBreakpoints(String serverAbsPath, BpLineAn
823883
* Seems we're not allowed to inspect the jdwp-native id, but we can attach our own
824884
*/
825885
private IBreakpoint[] __internal__idempotentBindBreakpoints(KlassMap klassMap, BpLineAndId[] lineInfo) {
826-
clearExistingBreakpoints(klassMap.sourceName.transformed);
827-
828-
final var replayable = new ArrayList<ReplayableCfBreakpointRequest>();
886+
final var replayable = replayableBreakpointRequestsByAbsPath_.computeIfAbsent(klassMap.sourceName.transformed, _z -> new HashSet<>());
829887
final var result = new ArrayList<IBreakpoint>();
830888

831889
for (int i = 0; i < lineInfo.length; ++i) {
@@ -864,7 +922,7 @@ private IBreakpoint[] __internal__idempotentBindBreakpoints(KlassMap klassMap, B
864922
* returns an array of the line numbers the old breakpoints were bound to
865923
*/
866924
private void clearExistingBreakpoints(String absPath) {
867-
var replayable = replayableBreakpointRequestsByAbsPath_.get(absPath);
925+
Set<ReplayableCfBreakpointRequest> replayable = replayableBreakpointRequestsByAbsPath_.get(absPath);
868926

869927
// "just do it" in all cases
870928
replayableBreakpointRequestsByAbsPath_.remove(absPath);
@@ -1030,7 +1088,9 @@ public String dumpAsJSON(int dapVariablesReference) {
10301088
public String[] getTrackedCanonicalFileNames() {
10311089
final var result = new ArrayList<String>();
10321090
for (var klassMap : klassMap_.values()) {
1033-
result.add(klassMap.sourceName.transformed);
1091+
for (var mapping : klassMap) {
1092+
result.add(mapping.sourceName.transformed);
1093+
}
10341094
}
10351095
return result.toArray(size -> new String[size]);
10361096
}

0 commit comments

Comments
 (0)