From 84d5ba68e00a0e1739e8d2bf2bf31226ccc9495e Mon Sep 17 00:00:00 2001 From: David Rogers Date: Mon, 18 Nov 2024 18:11:09 -0600 Subject: [PATCH] don't push frames while pushing frames --- .../src/main/java/luceedebug/Agent.java | 9 +- .../coreinject/CfValueDebuggerBridge.java | 8 +- .../luceedebug/coreinject/DebugManager.java | 94 +++-- .../luceedebug/coreinject/ExprEvaluator.java | 14 +- .../coreinject/frame/DebugFrame.java | 391 ++---------------- .../coreinject/frame/DummyFrame.java | 28 ++ .../luceedebug/coreinject/frame/Frame.java | 334 +++++++++++++++ 7 files changed, 467 insertions(+), 411 deletions(-) create mode 100644 luceedebug/src/main/java/luceedebug/coreinject/frame/DummyFrame.java create mode 100644 luceedebug/src/main/java/luceedebug/coreinject/frame/Frame.java diff --git a/luceedebug/src/main/java/luceedebug/Agent.java b/luceedebug/src/main/java/luceedebug/Agent.java index 0471941..c5c2e84 100644 --- a/luceedebug/src/main/java/luceedebug/Agent.java +++ b/luceedebug/src/main/java/luceedebug/Agent.java @@ -155,7 +155,6 @@ private static Map linearizedCoreInjectClasses() { result.put("luceedebug.coreinject.DebugManager$1", 0); result.put("luceedebug.coreinject.ClosureScopeLocalScopeAccessorShim", 0); result.put("luceedebug.coreinject.ComponentScopeMarkerTraitShim", 0); - result.put("luceedebug.coreinject.DebugFrame$FrameContext", 0); result.put("luceedebug.coreinject.LuceeVm$SteppingState", 0); result.put("luceedebug.coreinject.LuceeVm$KlassMap", 0); result.put("luceedebug.coreinject.LuceeVm$JdwpWorker", 0); @@ -165,14 +164,12 @@ private static Map linearizedCoreInjectClasses() { result.put("luceedebug.coreinject.CfValueDebuggerBridge$MarkerTrait", 0); result.put("luceedebug.coreinject.ValTracker", 0); result.put("luceedebug.coreinject.UnsafeUtils", 0); - result.put("luceedebug.coreinject.DebugFrame", 0); result.put("luceedebug.coreinject.CfValueDebuggerBridge$MarkerTrait$Scope", 0); result.put("luceedebug.coreinject.DebugManager$PageContextAndOutputStream", 0); result.put("luceedebug.coreinject.LuceeVm$ThreadMap", 0); result.put("luceedebug.coreinject.DebugManager", 0); result.put("luceedebug.coreinject.LuceeVm$JdwpStaticCallable", 0); result.put("luceedebug.coreinject.CfValueDebuggerBridge", 0); - result.put("luceedebug.coreinject.DebugFrame$FrameContext$SupplierOrNull", 0); result.put("luceedebug.coreinject.LuceeVm", 0); result.put("luceedebug.coreinject.ValTracker$CleanerRunner", 0); result.put("luceedebug.coreinject.ExprEvaluator", 0); @@ -180,6 +177,12 @@ private static Map linearizedCoreInjectClasses() { result.put("luceedebug.coreinject.ExprEvaluator$Evaluator", 0); result.put("luceedebug.coreinject.ExprEvaluator$Lucee6Evaluator", 1); result.put("luceedebug.coreinject.ExprEvaluator$Lucee5Evaluator", 1); + + result.put("luceedebug.coreinject.frame.DebugFrame", 0); + result.put("luceedebug.coreinject.frame.Frame", 1); + result.put("luceedebug.coreinject.frame.Frame$FrameContext", 1); + result.put("luceedebug.coreinject.frame.Frame$FrameContext$SupplierOrNull", 1); + result.put("luceedebug.coreinject.frame.DummyFrame", 1); return result; } diff --git a/luceedebug/src/main/java/luceedebug/coreinject/CfValueDebuggerBridge.java b/luceedebug/src/main/java/luceedebug/coreinject/CfValueDebuggerBridge.java index 586c2b8..d8c410e 100644 --- a/luceedebug/src/main/java/luceedebug/coreinject/CfValueDebuggerBridge.java +++ b/luceedebug/src/main/java/luceedebug/coreinject/CfValueDebuggerBridge.java @@ -15,7 +15,7 @@ import lucee.runtime.type.Array; import luceedebug.ICfValueDebuggerBridge; import luceedebug.IDebugEntity; -import luceedebug.coreinject.frame.DebugFrame; +import luceedebug.coreinject.frame.Frame; public class CfValueDebuggerBridge implements ICfValueDebuggerBridge { // Pin some ephemeral evaluated things so they don't get GC'd immediately. @@ -32,11 +32,11 @@ public static void pin(Object obj) { pinnedObjects.put(System.identityHashCode(obj), obj); } - private final DebugFrame frame; + private final Frame frame; public final Object obj; public final long id; - public CfValueDebuggerBridge(DebugFrame frame, Object obj) { + public CfValueDebuggerBridge(Frame frame, Object obj) { this.frame = Objects.requireNonNull(frame); this.obj = Objects.requireNonNull(obj); this.id = frame.valTracker.idempotentRegisterObject(obj).id; @@ -58,7 +58,7 @@ public Scope(Map scopelike) { /** * @maybeNull_which --> null means "any type" */ - public static IDebugEntity[] getAsDebugEntity(DebugFrame frame, Object obj, IDebugEntity.DebugEntityType maybeNull_which) { + public static IDebugEntity[] getAsDebugEntity(Frame frame, Object obj, IDebugEntity.DebugEntityType maybeNull_which) { return getAsDebugEntity(frame.valTracker, obj, maybeNull_which); } diff --git a/luceedebug/src/main/java/luceedebug/coreinject/DebugManager.java b/luceedebug/src/main/java/luceedebug/coreinject/DebugManager.java index cef158b..1391714 100644 --- a/luceedebug/src/main/java/luceedebug/coreinject/DebugManager.java +++ b/luceedebug/src/main/java/luceedebug/coreinject/DebugManager.java @@ -33,6 +33,7 @@ import luceedebug.IDebugFrame; import luceedebug.IDebugManager; import luceedebug.coreinject.frame.DebugFrame; +import luceedebug.coreinject.frame.Frame; public class DebugManager implements IDebugManager { @@ -328,36 +329,37 @@ synchronized private String doDumpAsJSON(PageContext pageContext, Object someDum } public Either> evaluate(Long frameID, String expr) { - final var frame = frameByFrameID.get(frameID); - if (frame != null) { - return doEvaluate(frame, expr) - .bimap( - err -> err, - ok -> { - // what about bool, Long, etc. ?... - if (ok == null) { - return Either.Right("null"); - } - else if (ok instanceof String) { - return Either.Right("\"" + ((String)ok).replaceAll("\"", "\\\"") + "\""); - } - else if (ok instanceof Number || ok instanceof Boolean) { - return Either.Right(ok.toString()); - } - else { - return Either.Left(frame.trackEvalResult(ok)); - } - } - ); - } - else { + final var zzzframe = frameByFrameID.get(frameID); + if (!(zzzframe instanceof Frame)) { return Either.Left("<>"); } + + Frame frame = (Frame)zzzframe; + + return doEvaluate(frame, expr) + .bimap( + err -> err, + ok -> { + // what about bool, Long, etc. ?... + if (ok == null) { + return Either.Right("null"); + } + else if (ok instanceof String) { + return Either.Right("\"" + ((String)ok).replaceAll("\"", "\\\"") + "\""); + } + else if (ok instanceof Number || ok instanceof Boolean) { + return Either.Right(ok.toString()); + } + else { + return Either.Left(frame.trackEvalResult(ok)); + } + } + ); } // concurrency here needs to be at the level of the DAP server? // does the DAP server do multiple concurrent requests ... ? ... it's all one socket so probably not ? ... well many inbound messages can be being serviced ... - private Either doEvaluate(DebugFrame frame, String expr) { + private Either doEvaluate(Frame frame, String expr) { try { return CompletableFuture .supplyAsync( @@ -392,10 +394,18 @@ public boolean evaluateAsBooleanForConditionalBreakpoint(Thread thread, String e if (stack.isEmpty()) { return false; } - return doEvaluateAsBoolean(stack.get(stack.size() - 1), expr); + + DebugFrame frame = stack.get(stack.size() - 1); + + if (frame instanceof Frame) { + return doEvaluateAsBoolean((Frame)frame, expr); + } + else { + return false; + } } - private boolean doEvaluateAsBoolean(DebugFrame frame, String expr) { + private boolean doEvaluateAsBoolean(Frame frame, String expr) { try { return CompletableFuture .supplyAsync( @@ -482,7 +492,7 @@ synchronized public IDebugFrame[] getCfStack(Thread thread) { if (stack == null) { System.out.println("getCfStack called, frames was null, frames is " + cfStackByThread + ", passed thread was " + thread); System.out.println(" thread=" + thread + " this=" + this); - return new DebugFrame[0]; + return new Frame[0]; } ArrayList result = new ArrayList<>(); @@ -501,7 +511,7 @@ synchronized public IDebugFrame[] getCfStack(Thread thread) { } } - return result.toArray(new DebugFrame[result.size()]); + return result.toArray(new Frame[result.size()]); } static class CfStepRequest { @@ -570,9 +580,12 @@ public void luceedebug_stepNotificationEntry_step(int lineNumber) { if (request == null) { return; } - else { + else if (frame instanceof Frame) { request.__debug__steps++; - maybeNotifyOfStepCompletion(currentThread, frame, request, minDistanceToLuceedebugStepNotificationEntryFrame + 1, System.nanoTime()); + maybeNotifyOfStepCompletion(currentThread, (Frame) frame, request, minDistanceToLuceedebugStepNotificationEntryFrame + 1, System.nanoTime()); + } + else { + // no-op } } @@ -596,13 +609,16 @@ public void luceedebug_stepNotificationEntry_stepAfterCompletedUdfCall() { if (request == null) { return; } - else { + else if (frame instanceof Frame) { request.__debug__steps++; - maybeNotifyOfStepCompletion(currentThread, frame, request, minDistanceToLuceedebugStepNotificationEntryFrame + 1, System.nanoTime()); + maybeNotifyOfStepCompletion(currentThread, (Frame)frame, request, minDistanceToLuceedebugStepNotificationEntryFrame + 1, System.nanoTime()); + } + else { + // no-op } } - private void maybeNotifyOfStepCompletion(Thread currentThread, DebugFrame frame, CfStepRequest request, int minDistanceToLuceedebugStepNotificationEntryFrame, long start) { + private void maybeNotifyOfStepCompletion(Thread currentThread, Frame frame, CfStepRequest request, int minDistanceToLuceedebugStepNotificationEntryFrame, long start) { if (frame.isUdfDefaultValueInitFrame && !config_.getStepIntoUdfDefaultValueInitFrames()) { return; } @@ -683,11 +699,7 @@ private DebugFrame maybe_pushCfFrame_worker(PageContext pageContext, String sour final int depth = stack.size(); // first frame is frame 0, and prior to pushing the first frame the stack is length 0; next frame is frame 1, and prior to pushing it the stack is of length 1, ... - final DebugFrame frame = DebugFrame.maybeMakeFrame(sourceFilePath, depth, valTracker, pageContext); - - if (frame == null) { - return null; - } + final DebugFrame frame = DebugFrame.makeFrame(sourceFilePath, depth, valTracker, pageContext); stack.add(frame); @@ -703,11 +715,9 @@ private DebugFrame maybe_pushCfFrame_worker(PageContext pageContext, String sour public void pushCfFunctionDefaultValueInitializationFrame(lucee.runtime.PageContext pageContext, String sourceFilePath) { DebugFrame frame = maybe_pushCfFrame_worker(pageContext, sourceFilePath); - if (frame == null) { - return; + if (frame instanceof Frame) { + ((Frame)frame).isUdfDefaultValueInitFrame = true; } - - frame.isUdfDefaultValueInitFrame = true; } public void popCfFrame() { diff --git a/luceedebug/src/main/java/luceedebug/coreinject/ExprEvaluator.java b/luceedebug/src/main/java/luceedebug/coreinject/ExprEvaluator.java index 711cac2..962c7cc 100644 --- a/luceedebug/src/main/java/luceedebug/coreinject/ExprEvaluator.java +++ b/luceedebug/src/main/java/luceedebug/coreinject/ExprEvaluator.java @@ -8,7 +8,7 @@ import lucee.runtime.PageContext; import luceedebug.Either; -import luceedebug.coreinject.frame.DebugFrame; +import luceedebug.coreinject.frame.Frame; import static luceedebug.coreinject.Utils.terminate; @@ -27,7 +27,7 @@ class ExprEvaluator { } } - public static Either eval(DebugFrame frame, String expr) { + public static Either eval(Frame frame, String expr) { return lucee5 .map(v -> v.eval(frame, expr)) .or(() -> lucee6.map(v -> v.eval(frame, expr))) @@ -52,9 +52,9 @@ static protected String getEvaluatableSourceText(String expr) { + ""; } - protected abstract void evalIntoVariablesScope(DebugFrame frame, String expr) throws Throwable; + protected abstract void evalIntoVariablesScope(Frame frame, String expr) throws Throwable; - public Either eval(DebugFrame frame, String expr) { + public Either eval(Frame frame, String expr) { try { evalIntoVariablesScope(frame, expr); var obj = consumeResult(frame); @@ -68,7 +68,7 @@ static protected String getEvaluatableSourceText(String expr) { /** * get the eval'd result out of the frame's variables scope and then delete it from the variables scope. */ - private Object consumeResult(DebugFrame frame) throws Throwable { + private Object consumeResult(Frame frame) throws Throwable { Object evalResult = UnsafeUtils.deprecatedScopeGet(frame.getFrameContext().variables, resultName); frame.getFrameContext().variables.remove(resultName); return evalResult; @@ -108,7 +108,7 @@ private static class Lucee5Evaluator extends Evaluator { this.methodHandle = methodHandle; } - protected void evalIntoVariablesScope(DebugFrame frame, String expr) throws Throwable { + protected void evalIntoVariablesScope(Frame frame, String expr) throws Throwable { methodHandle.invoke( /*PageContext pc*/ frame.getFrameContext().pageContext, /*String cfml*/ Evaluator.getEvaluatableSourceText(expr), @@ -148,7 +148,7 @@ private static class Lucee6Evaluator extends Evaluator { this.methodHandle = methodHandle; } - protected void evalIntoVariablesScope(DebugFrame frame, String expr) throws Throwable { + protected void evalIntoVariablesScope(Frame frame, String expr) throws Throwable { methodHandle.invoke( /*PageContext pc*/ frame.getFrameContext().pageContext, /*String cfml*/ Evaluator.getEvaluatableSourceText(expr), diff --git a/luceedebug/src/main/java/luceedebug/coreinject/frame/DebugFrame.java b/luceedebug/src/main/java/luceedebug/coreinject/frame/DebugFrame.java index a6da913..25d6a0a 100644 --- a/luceedebug/src/main/java/luceedebug/coreinject/frame/DebugFrame.java +++ b/luceedebug/src/main/java/luceedebug/coreinject/frame/DebugFrame.java @@ -1,355 +1,36 @@ -package luceedebug.coreinject.frame; - -import lucee.runtime.PageContext; -import lucee.runtime.PageContextImpl; -import lucee.runtime.type.scope.LocalNotSupportedScope; - -import lucee.runtime.type.Collection; - -import java.util.ArrayList; -import java.util.IdentityHashMap; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.Objects; -import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Supplier; - -import com.google.common.collect.MapMaker; - -import luceedebug.*; -import luceedebug.coreinject.CfValueDebuggerBridge; -import luceedebug.coreinject.ClosureScopeLocalScopeAccessorShim; -import luceedebug.coreinject.DebugEntity; -import luceedebug.coreinject.UnsafeUtils; -import luceedebug.coreinject.ValTracker; -import luceedebug.coreinject.CfValueDebuggerBridge.MarkerTrait; -import luceedebug.coreinject.CfValueDebuggerBridge.MarkerTrait.Scope; - -public class DebugFrame implements IDebugFrame { - static private AtomicLong nextId = new AtomicLong(0); - - /** - * It's not 100% clear that our instrumentation to walk captured closure scopes will always be valid across all class loaders, - * and we assume that if it fails once, we should disable it across the entire program. - */ - static private boolean closureScopeGloballyDisabled = false; - - public final ValTracker valTracker; - - final private FrameContext frameContext_; - final private String sourceFilePath; - final private long id; - final private String name; - final private int depth; // 0 is first frame in stack, 1 is next, ... - private int line = 0; // initially unknown, until first step notification - - /** - * True if this cf frame's actual java method is "udfDefaultValue", see lucee source - * This should be final and init'd via constructor but it's not a pressing issue. - */ - public boolean isUdfDefaultValueInitFrame = false; - - public String getSourceFilePath() { return sourceFilePath; }; - public long getId() { return id; } - public String getName() { return name; } - public int getDepth() { return depth; } - public int getLine() { return line; } - public void setLine(int line) { this.line = line; } - - // lazy initialized on request for scopes - // This is "scopes, wrapped with trackable IDs, which are expensive to create and cleanup" - private LinkedHashMap scopes_ = null; - - // the results of evaluating complex expressions need to be kept alive for the entirety of the frame - // these should be made gc'able when this frame is collected - // We might want to place these results somewhere that is kept alive for the whole request? - private ArrayList refsToKeepAlive_ = new ArrayList<>(); - void pin(Object obj) { - refsToKeepAlive_.add(obj); - } - - static ThreadLocal isPushingFrame = ThreadLocal.withInitial(() -> false); - - // hold strong refs to scopes, because PageContext will swap them out as frames change (variables, local, this) - // (application, server and etc. maybe could be held as globals) - // We don't want to construct tracked refs to them until a debugger asks for them, because it is expensive - // to create and clean up references for every pushed frame, especially if that frame isn't ever inspected in a debugger. - // This should be valid for the entirety of the frame, and should the frame should be always be disposed of at the end of the actual cf frame. - // - // lifetime: we shouldn't hold onto a frame longer than the engine holds onto the "frame" - // (where frame there is in air quotes because the engine doesn't track explicit frames) - // We want to not make this un-GC'able at the time the engine assumes it's GC'able. - // This should be doable by virtue of our frames being popped and released immediately before - // the engine is truly do with its "frame". Fallback here would be use a WeakRef<> but it doesn't - // seem necessary. - // - public static class FrameContext { - final public PageContext pageContext; - - public final lucee.runtime.type.scope.Scope application; - public final lucee.runtime.type.scope.Argument arguments; - public final lucee.runtime.type.scope.Scope form; - public final lucee.runtime.type.scope.Local local; - public final lucee.runtime.type.scope.Scope request; - public final lucee.runtime.type.scope.Scope session; - public final lucee.runtime.type.scope.Scope server; - public final lucee.runtime.type.scope.Scope url; - public final lucee.runtime.type.scope.Variables variables; - // n.b. the `this` scope does not derive from Scope - public final lucee.runtime.type.Struct this_; - public final lucee.runtime.type.scope.Scope static_; - - // lazy init because it (might?) be expensive to walk scope chains eagerly every frame - private ArrayList capturedScopeChain = null; - - static private final ConcurrentMap activeFrameLockByPageContext = new MapMaker() - .weakKeys() - .makeMap(); - - // - // Note that some of these `getScopeOrNull` calls need additional guards, to prevent from throwing - // expensive exceptions on literally every frame, e.g. if a scope is disabled by the engine and trying to touch it - // throws an ExpressionException. - // - private FrameContext(PageContext pageContext) { - this.pageContext = pageContext; - this.application = getScopelikeOrNull(() -> pageContext.applicationScope()); - this.arguments = getScopelikeOrNull(() -> pageContext.argumentsScope()); - this.form = getScopelikeOrNull(() -> pageContext.formScope()); - this.local = getScopelikeOrNull(() -> pageContext.localScope()); - this.request = getScopelikeOrNull(() -> pageContext.requestScope()); - this.session = getScopelikeOrNull(() -> pageContext.getApplicationContext().isSetSessionManagement() ? pageContext.sessionScope() : null); - this.server = getScopelikeOrNull(() -> pageContext.serverScope()); - this.url = getScopelikeOrNull(() -> pageContext.urlScope()); - this.variables = getScopelikeOrNull(() -> pageContext.variablesScope()); - this.this_ = getScopelikeOrNull(() -> { - // there is also `PageContextImpl.thisGet()` but it can create a `this` property on the variables scope, which seems like - // something we don't want to do, since it mutates the user's scopes instead of just reading from them. - if (this.variables instanceof lucee.runtime.ComponentScope) { - // The `this` scope IS the component, bound to the variables scope that is an instanceof ComponentScope - // (which means ComponentScope is a variables scope containing a THIS scope, rather than ComponentScope IS the this scope) - // Alternatively we could just lookup the `this` property on `variables`. - return ((lucee.runtime.ComponentScope)this.variables).getComponent(); - } - else if (this.variables instanceof lucee.runtime.type.scope.ClosureScope) { - // A closure scope is a variables scope wrapper containing a variable scope. - // Probably we could test here for if the closureScope contains a component scope, but just looking for `this` seems to be fine. - return (lucee.runtime.type.Struct)UnsafeUtils.deprecatedScopeGet(this.variables, "this"); - } - else { - return null; - } - }); - - // If we have a `this` scope, meaning we are in a component, then we should have a static scope. - this.static_ = this.this_ instanceof lucee.runtime.Component ? ((lucee.runtime.Component)this.this_).staticScope() : null; - } - - public ArrayList getCapturedScopeChain() { - if (capturedScopeChain == null) { - capturedScopeChain = getCapturedScopeChain(variables); - } - return capturedScopeChain; - } - - private static ArrayList getCapturedScopeChain(lucee.runtime.type.scope.Scope variables) { - if (variables instanceof lucee.runtime.type.scope.ClosureScope) { - final var setLike_seen = new IdentityHashMap<>(); - final var result = new ArrayList(); - var scope = variables; - while (scope instanceof lucee.runtime.type.scope.ClosureScope) { - final var captured = (lucee.runtime.type.scope.ClosureScope)scope; - if (setLike_seen.containsKey(captured)) { - break; - } - else { - setLike_seen.put(captured, true); - } - result.add(captured); - scope = captured.getVariables(); - } - return result; - } - else { - return new ArrayList<>(); - } - } - - interface SupplierOrNull { - T get() throws Throwable; - } - - // sometimes trying to get a scope throws, in which case we get null - // scopes that are "garbage" scopes ("LocalNotSupportedScope") should be filtered away elsewhere - // we especially are interested in when we swap out scopes during expression evaluation that we restore the scopes - // as they were prior to; which might be troublesome if "getting a scope throws so we return null, but it doesn't make sense to restore the scope to null" - private T getScopelikeOrNull(SupplierOrNull f) { - try { - return f.get(); - } - catch(Throwable e) { - return null; - } - } - - // if we're mutating some page context's frame information in place, we should only do it on one thread at a time. - static private T withPageContextLock(PageContext pageContext, Supplier f) { - // lazy create the lock if it doesn't exist yet - synchronized(activeFrameLockByPageContext.computeIfAbsent(pageContext, (_x) -> new Object())) { - return f.get(); - } - } - - /** - * "real" frames are swapped-out in place inside the engine, so there's just one page context that has its - * current context mutated on function enter/exit. To evaluate an expression inside of some frame context, - * we need to replace the page context's relevant scopes with the ones for "this" frame, perform the evaluation, - * and then restore everything we swapped out. - */ - public T doWorkInThisFrame(Supplier f) { - return withPageContextLock(pageContext, () -> { - final var saved_argumentsScope = getScopelikeOrNull(() -> pageContext.argumentsScope()); - final var saved_localScope = getScopelikeOrNull(() -> pageContext.localScope()); - final var saved_variablesScope = getScopelikeOrNull(() -> pageContext.variablesScope()); - try { - pageContext.setFunctionScopes(local, arguments); - pageContext.setVariablesScope(variables); - return f.get(); - } - finally { - pageContext.setVariablesScope(saved_variablesScope); - pageContext.setFunctionScopes(saved_localScope, saved_argumentsScope); - } - }); - } - } - - // DebugFrame >: DummyFrame | Frame - static private DebugFrame dummyFrame = new DebugFrame("null", 0, null, null); - - static public DebugFrame maybeMakeFrame(String sourceFilePath, int depth, ValTracker valTracker, PageContext pageContext) { - if (isPushingFrame.get()) { - return null; - } - else { - try { - isPushingFrame.set(true); - return new DebugFrame(sourceFilePath, depth, valTracker, pageContext, DebugFrame.tryGetFrameName(pageContext)); - } - finally { - isPushingFrame.set(false); - } - } - } - - private DebugFrame(String sourceFilePath, int depth, ValTracker valTracker, PageContext pageContext) { - this(sourceFilePath, depth, valTracker, pageContext, DebugFrame.tryGetFrameName(pageContext)); - } - - private DebugFrame(String sourceFilePath, int depth, ValTracker valTracker, PageContext pageContext, String name) { - this.frameContext_ = new FrameContext(pageContext); - this.sourceFilePath = Objects.requireNonNull(sourceFilePath); - this.valTracker = Objects.requireNonNull(valTracker); - this.id = nextId.incrementAndGet(); - this.name = name; - this.depth = depth; - } - - private static String tryGetFrameName(PageContext pageContext) { - String frameName = "??"; - try { - final PageContextImpl pageContextImpl = (PageContextImpl)pageContext; - final Collection.Key key = pageContextImpl.getActiveUDFCalledName(); - if (key != null) { - frameName = key.getString(); - } - } - catch (Throwable e) { - // discard, cast was bad for some reason? - } - return frameName; - } - - private void checkedPutScopeRef(String name, Map scope) { - if (scope != null && !(scope instanceof LocalNotSupportedScope)) { - var v = new MarkerTrait.Scope(scope); - pin(v); - scopes_.put(name, new CfValueDebuggerBridge(this, v)); - } - } - - private void lazyInitScopeRefs() { - if (scopes_ != null) { - // already init'd - return; - } - - scopes_ = new LinkedHashMap<>(); - checkedPutScopeRef("application", frameContext_.application); - checkedPutScopeRef("arguments", frameContext_.arguments); - checkedPutScopeRef("form", frameContext_.form); - checkedPutScopeRef("local", frameContext_.local); - checkedPutScopeRef("request", frameContext_.request); - checkedPutScopeRef("session", frameContext_.session); - checkedPutScopeRef("static", frameContext_.static_); - checkedPutScopeRef("server", frameContext_.server); - checkedPutScopeRef("this", frameContext_.this_); - checkedPutScopeRef("url", frameContext_.url); - checkedPutScopeRef("variables", frameContext_.variables); - - if (!closureScopeGloballyDisabled) { - final var scopeChain = frameContext_.getCapturedScopeChain(); - final int captureChainLen = scopeChain.size(); - try { - for (int i = 0; i < captureChainLen; i++) { - // this should always succeed, there's no casting into a luceedebug shim type - checkedPutScopeRef("captured arguments " + i, scopeChain.get(i).getArgument()); - // this could potentially fail with a class cast exception - checkedPutScopeRef("captured local " + i, ((ClosureScopeLocalScopeAccessorShim)scopeChain.get(i)).getLocalScope()); - } - } - catch (ClassCastException e) { - // We'll be left with possibly some capture scopes in the list this time around, - // but all subsequent calls to this method will be guarded by this assignment. - closureScopeGloballyDisabled = true; - return; - } - } - } - - /** - * for debugger-internal use, e.g. in watch expressions - */ - public FrameContext getFrameContext() { - return frameContext_; - } - - /** - * for direct DAP use - */ - public IDebugEntity[] getScopes() { - lazyInitScopeRefs(); - IDebugEntity[] result = new DebugEntity[scopes_.size()]; - int i = 0; - for (var kv : scopes_.entrySet()) { - String name = kv.getKey(); - CfValueDebuggerBridge entityRef = kv.getValue(); - var entity = new DebugEntity(); - entity.name = name; - entity.namedVariables = entityRef.getNamedVariablesCount(); - entity.indexedVariables = entityRef.getIndexedVariablesCount(); - entity.expensive = true; - entity.variablesReference = entityRef.id; - result[i] = entity; - i += 1; - } - return result; - } - - public CfValueDebuggerBridge trackEvalResult(Object obj) { - var v = new CfValueDebuggerBridge(this, obj); - CfValueDebuggerBridge.pin(obj); - return v; - } -} +package luceedebug.coreinject.frame; + +import lucee.runtime.PageContext; +import luceedebug.IDebugFrame; +import luceedebug.coreinject.ValTracker; + +/** + * Should be a sealed class, subtypes are: + * - Frame + * - DummyFrame + */ +public abstract class DebugFrame implements IDebugFrame { + // https://github.com/softwareCobbler/luceedebug/issues/68 + // generating a debug frame involves calling into lucee engine code to grab scopes. + // Lucee can do anything it wants there, and at least when reading from the session scope + // to deserialize cfcs that had been serialized, can end up invoking more coldfusion code (i.e. calling their + // pseudoconstructors), which in turn can push more frames, and we want to track those frames, which can push + // more frames ... and so on. So when we push a frame, we need to know if we are "already pushing a frame", + // and if so, we just return some dummy frame which is guranteed to NOT schedule more work. + static private ThreadLocal isPushingFrame = ThreadLocal.withInitial(() -> false); + + static public DebugFrame makeFrame(String sourceFilePath, int depth, ValTracker valTracker, PageContext pageContext) { + if (isPushingFrame.get()) { + return DummyFrame.get(); + } + else { + try { + isPushingFrame.set(true); + return new Frame(sourceFilePath, depth, valTracker, pageContext); + } + finally { + isPushingFrame.set(false); + } + } + } +} diff --git a/luceedebug/src/main/java/luceedebug/coreinject/frame/DummyFrame.java b/luceedebug/src/main/java/luceedebug/coreinject/frame/DummyFrame.java new file mode 100644 index 0000000..82d7818 --- /dev/null +++ b/luceedebug/src/main/java/luceedebug/coreinject/frame/DummyFrame.java @@ -0,0 +1,28 @@ +package luceedebug.coreinject.frame; + +import luceedebug.IDebugEntity; + +/** + * DummyFrame is just a placeholder for when we wanted to push a frame + * but were unable to generate one. See notes on `DebugFrame`. + */ +public class DummyFrame extends DebugFrame { + private static DummyFrame instance = new DummyFrame(); + private DummyFrame() {} + + public static DummyFrame get() { + return instance; + } + + private static T fail() { + throw new RuntimeException("Methods on 'DummyFrame' should never be called."); + } + + public String getSourceFilePath() { return fail(); }; + public long getId() { return fail(); }; + public String getName() { return fail(); }; + public int getDepth() { return fail(); }; + public int getLine() { return fail(); }; + public void setLine(int line) { fail(); }; + public IDebugEntity[] getScopes() { return fail(); } +} diff --git a/luceedebug/src/main/java/luceedebug/coreinject/frame/Frame.java b/luceedebug/src/main/java/luceedebug/coreinject/frame/Frame.java new file mode 100644 index 0000000..6936783 --- /dev/null +++ b/luceedebug/src/main/java/luceedebug/coreinject/frame/Frame.java @@ -0,0 +1,334 @@ +package luceedebug.coreinject.frame; + +import lucee.runtime.PageContext; +import lucee.runtime.PageContextImpl; +import lucee.runtime.type.scope.LocalNotSupportedScope; + +import lucee.runtime.type.Collection; + +import java.util.ArrayList; +import java.util.IdentityHashMap; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Supplier; + +import com.google.common.collect.MapMaker; + +import luceedebug.*; +import luceedebug.coreinject.CfValueDebuggerBridge; +import luceedebug.coreinject.ClosureScopeLocalScopeAccessorShim; +import luceedebug.coreinject.DebugEntity; +import luceedebug.coreinject.UnsafeUtils; +import luceedebug.coreinject.ValTracker; +import luceedebug.coreinject.CfValueDebuggerBridge.MarkerTrait; + +public class Frame extends DebugFrame { + static private AtomicLong nextId = new AtomicLong(0); + + /** + * It's not 100% clear that our instrumentation to walk captured closure scopes will always be valid across all class loaders, + * and we assume that if it fails once, we should disable it across the entire program. + */ + static private boolean closureScopeGloballyDisabled = false; + + public final ValTracker valTracker; + + final private FrameContext frameContext_; + final private String sourceFilePath; + final private long id; + final private String name; + final private int depth; // 0 is first frame in stack, 1 is next, ... + private int line = 0; // initially unknown, until first step notification + + /** + * True if this cf frame's actual java method is "udfDefaultValue", see lucee source + * This should be final and init'd via constructor but it's not a pressing issue. + */ + public boolean isUdfDefaultValueInitFrame = false; + + public String getSourceFilePath() { return sourceFilePath; }; + public long getId() { return id; } + public String getName() { return name; } + public int getDepth() { return depth; } + public int getLine() { return line; } + public void setLine(int line) { this.line = line; } + + // lazy initialized on request for scopes + // This is "scopes, wrapped with trackable IDs, which are expensive to create and cleanup" + private LinkedHashMap scopes_ = null; + + // the results of evaluating complex expressions need to be kept alive for the entirety of the frame + // these should be made gc'able when this frame is collected + // We might want to place these results somewhere that is kept alive for the whole request? + private ArrayList refsToKeepAlive_ = new ArrayList<>(); + void pin(Object obj) { + refsToKeepAlive_.add(obj); + } + + // hold strong refs to scopes, because PageContext will swap them out as frames change (variables, local, this) + // (application, server and etc. maybe could be held as globals) + // We don't want to construct tracked refs to them until a debugger asks for them, because it is expensive + // to create and clean up references for every pushed frame, especially if that frame isn't ever inspected in a debugger. + // This should be valid for the entirety of the frame, and should the frame should be always be disposed of at the end of the actual cf frame. + // + // lifetime: we shouldn't hold onto a frame longer than the engine holds onto the "frame" + // (where frame there is in air quotes because the engine doesn't track explicit frames) + // We want to not make this un-GC'able at the time the engine assumes it's GC'able. + // This should be doable by virtue of our frames being popped and released immediately before + // the engine is truly do with its "frame". Fallback here would be use a WeakRef<> but it doesn't + // seem necessary. + // + public static class FrameContext { + final public PageContext pageContext; + + public final lucee.runtime.type.scope.Scope application; + public final lucee.runtime.type.scope.Argument arguments; + public final lucee.runtime.type.scope.Scope form; + public final lucee.runtime.type.scope.Local local; + public final lucee.runtime.type.scope.Scope request; + public final lucee.runtime.type.scope.Scope session; + public final lucee.runtime.type.scope.Scope server; + public final lucee.runtime.type.scope.Scope url; + public final lucee.runtime.type.scope.Variables variables; + // n.b. the `this` scope does not derive from Scope + public final lucee.runtime.type.Struct this_; + public final lucee.runtime.type.scope.Scope static_; + + // lazy init because it (might?) be expensive to walk scope chains eagerly every frame + private ArrayList capturedScopeChain = null; + + static private final ConcurrentMap activeFrameLockByPageContext = new MapMaker() + .weakKeys() + .makeMap(); + + // + // Note that some of these `getScopeOrNull` calls need additional guards, to prevent from throwing + // expensive exceptions on literally every frame, e.g. if a scope is disabled by the engine and trying to touch it + // throws an ExpressionException. + // + private FrameContext(PageContext pageContext) { + this.pageContext = pageContext; + this.application = getScopelikeOrNull(() -> pageContext.applicationScope()); + this.arguments = getScopelikeOrNull(() -> pageContext.argumentsScope()); + this.form = getScopelikeOrNull(() -> pageContext.formScope()); + this.local = getScopelikeOrNull(() -> pageContext.localScope()); + this.request = getScopelikeOrNull(() -> pageContext.requestScope()); + this.session = getScopelikeOrNull(() -> pageContext.getApplicationContext().isSetSessionManagement() ? pageContext.sessionScope() : null); + this.server = getScopelikeOrNull(() -> pageContext.serverScope()); + this.url = getScopelikeOrNull(() -> pageContext.urlScope()); + this.variables = getScopelikeOrNull(() -> pageContext.variablesScope()); + this.this_ = getScopelikeOrNull(() -> { + // there is also `PageContextImpl.thisGet()` but it can create a `this` property on the variables scope, which seems like + // something we don't want to do, since it mutates the user's scopes instead of just reading from them. + if (this.variables instanceof lucee.runtime.ComponentScope) { + // The `this` scope IS the component, bound to the variables scope that is an instanceof ComponentScope + // (which means ComponentScope is a variables scope containing a THIS scope, rather than ComponentScope IS the this scope) + // Alternatively we could just lookup the `this` property on `variables`. + return ((lucee.runtime.ComponentScope)this.variables).getComponent(); + } + else if (this.variables instanceof lucee.runtime.type.scope.ClosureScope) { + // A closure scope is a variables scope wrapper containing a variable scope. + // Probably we could test here for if the closureScope contains a component scope, but just looking for `this` seems to be fine. + return (lucee.runtime.type.Struct)UnsafeUtils.deprecatedScopeGet(this.variables, "this"); + } + else { + return null; + } + }); + + // If we have a `this` scope, meaning we are in a component, then we should have a static scope. + this.static_ = this.this_ instanceof lucee.runtime.Component ? ((lucee.runtime.Component)this.this_).staticScope() : null; + } + + public ArrayList getCapturedScopeChain() { + if (capturedScopeChain == null) { + capturedScopeChain = getCapturedScopeChain(variables); + } + return capturedScopeChain; + } + + private static ArrayList getCapturedScopeChain(lucee.runtime.type.scope.Scope variables) { + if (variables instanceof lucee.runtime.type.scope.ClosureScope) { + final var setLike_seen = new IdentityHashMap<>(); + final var result = new ArrayList(); + var scope = variables; + while (scope instanceof lucee.runtime.type.scope.ClosureScope) { + final var captured = (lucee.runtime.type.scope.ClosureScope)scope; + if (setLike_seen.containsKey(captured)) { + break; + } + else { + setLike_seen.put(captured, true); + } + result.add(captured); + scope = captured.getVariables(); + } + return result; + } + else { + return new ArrayList<>(); + } + } + + interface SupplierOrNull { + T get() throws Throwable; + } + + // sometimes trying to get a scope throws, in which case we get null + // scopes that are "garbage" scopes ("LocalNotSupportedScope") should be filtered away elsewhere + // we especially are interested in when we swap out scopes during expression evaluation that we restore the scopes + // as they were prior to; which might be troublesome if "getting a scope throws so we return null, but it doesn't make sense to restore the scope to null" + private T getScopelikeOrNull(SupplierOrNull f) { + try { + return f.get(); + } + catch(Throwable e) { + return null; + } + } + + // if we're mutating some page context's frame information in place, we should only do it on one thread at a time. + static private T withPageContextLock(PageContext pageContext, Supplier f) { + // lazy create the lock if it doesn't exist yet + synchronized(activeFrameLockByPageContext.computeIfAbsent(pageContext, (_x) -> new Object())) { + return f.get(); + } + } + + /** + * "real" frames are swapped-out in place inside the engine, so there's just one page context that has its + * current context mutated on function enter/exit. To evaluate an expression inside of some frame context, + * we need to replace the page context's relevant scopes with the ones for "this" frame, perform the evaluation, + * and then restore everything we swapped out. + */ + public T doWorkInThisFrame(Supplier f) { + return withPageContextLock(pageContext, () -> { + final var saved_argumentsScope = getScopelikeOrNull(() -> pageContext.argumentsScope()); + final var saved_localScope = getScopelikeOrNull(() -> pageContext.localScope()); + final var saved_variablesScope = getScopelikeOrNull(() -> pageContext.variablesScope()); + try { + pageContext.setFunctionScopes(local, arguments); + pageContext.setVariablesScope(variables); + return f.get(); + } + finally { + pageContext.setVariablesScope(saved_variablesScope); + pageContext.setFunctionScopes(saved_localScope, saved_argumentsScope); + } + }); + } + } + + Frame(String sourceFilePath, int depth, ValTracker valTracker, PageContext pageContext) { + this(sourceFilePath, depth, valTracker, pageContext, Frame.tryGetFrameName(pageContext)); + } + + private Frame(String sourceFilePath, int depth, ValTracker valTracker, PageContext pageContext, String name) { + this.frameContext_ = new FrameContext(pageContext); + this.sourceFilePath = Objects.requireNonNull(sourceFilePath); + this.valTracker = Objects.requireNonNull(valTracker); + this.id = nextId.incrementAndGet(); + this.name = name; + this.depth = depth; + } + + private static String tryGetFrameName(PageContext pageContext) { + String frameName = "??"; + try { + final PageContextImpl pageContextImpl = (PageContextImpl)pageContext; + final Collection.Key key = pageContextImpl.getActiveUDFCalledName(); + if (key != null) { + frameName = key.getString(); + } + } + catch (Throwable e) { + // discard, cast was bad for some reason? + } + return frameName; + } + + private void checkedPutScopeRef(String name, Map scope) { + if (scope != null && !(scope instanceof LocalNotSupportedScope)) { + var v = new MarkerTrait.Scope(scope); + pin(v); + scopes_.put(name, new CfValueDebuggerBridge(this, v)); + } + } + + private void lazyInitScopeRefs() { + if (scopes_ != null) { + // already init'd + return; + } + + scopes_ = new LinkedHashMap<>(); + checkedPutScopeRef("application", frameContext_.application); + checkedPutScopeRef("arguments", frameContext_.arguments); + checkedPutScopeRef("form", frameContext_.form); + checkedPutScopeRef("local", frameContext_.local); + checkedPutScopeRef("request", frameContext_.request); + checkedPutScopeRef("session", frameContext_.session); + checkedPutScopeRef("static", frameContext_.static_); + checkedPutScopeRef("server", frameContext_.server); + checkedPutScopeRef("this", frameContext_.this_); + checkedPutScopeRef("url", frameContext_.url); + checkedPutScopeRef("variables", frameContext_.variables); + + if (!closureScopeGloballyDisabled) { + final var scopeChain = frameContext_.getCapturedScopeChain(); + final int captureChainLen = scopeChain.size(); + try { + for (int i = 0; i < captureChainLen; i++) { + // this should always succeed, there's no casting into a luceedebug shim type + checkedPutScopeRef("captured arguments " + i, scopeChain.get(i).getArgument()); + // this could potentially fail with a class cast exception + checkedPutScopeRef("captured local " + i, ((ClosureScopeLocalScopeAccessorShim)scopeChain.get(i)).getLocalScope()); + } + } + catch (ClassCastException e) { + // We'll be left with possibly some capture scopes in the list this time around, + // but all subsequent calls to this method will be guarded by this assignment. + closureScopeGloballyDisabled = true; + return; + } + } + } + + /** + * for debugger-internal use, e.g. in watch expressions + */ + public FrameContext getFrameContext() { + return frameContext_; + } + + /** + * for direct DAP use + */ + public IDebugEntity[] getScopes() { + lazyInitScopeRefs(); + IDebugEntity[] result = new DebugEntity[scopes_.size()]; + int i = 0; + for (var kv : scopes_.entrySet()) { + String name = kv.getKey(); + CfValueDebuggerBridge entityRef = kv.getValue(); + var entity = new DebugEntity(); + entity.name = name; + entity.namedVariables = entityRef.getNamedVariablesCount(); + entity.indexedVariables = entityRef.getIndexedVariablesCount(); + entity.expensive = true; + entity.variablesReference = entityRef.id; + result[i] = entity; + i += 1; + } + return result; + } + + public CfValueDebuggerBridge trackEvalResult(Object obj) { + var v = new CfValueDebuggerBridge(this, obj); + CfValueDebuggerBridge.pin(obj); + return v; + } +}