From ecc009f10d9c2fc95d558d9baab8814fba928b20 Mon Sep 17 00:00:00 2001 From: Zac Spitzer Date: Sat, 2 May 2026 19:46:17 +0200 Subject: [PATCH 1/2] LDEV-6298 v2 share flyweight accessors via BoundUDF wrap + DCL fix for concurrent addX --- .../java/lucee/runtime/ComponentImpl.java | 46 ++- .../lucee/runtime/ComponentScopeShadow.java | 4 + .../java/lucee/runtime/type/BoundUDF.java | 280 ++++++++++++++++++ .../lucee/runtime/type/UDFAddProperty.java | 19 +- .../lucee/runtime/type/UDFGSProperty.java | 10 +- .../lucee/runtime/type/UDFGetterProperty.java | 17 +- .../lucee/runtime/type/UDFHasProperty.java | 20 +- .../lucee/runtime/type/UDFRemoveProperty.java | 11 +- .../lucee/runtime/type/UDFSetterProperty.java | 42 +-- test/general/Accessors.cfc | 40 ++- 10 files changed, 392 insertions(+), 97 deletions(-) create mode 100644 core/src/main/java/lucee/runtime/type/BoundUDF.java diff --git a/core/src/main/java/lucee/runtime/ComponentImpl.java b/core/src/main/java/lucee/runtime/ComponentImpl.java index e7fd2b6c770..d3143c737dc 100755 --- a/core/src/main/java/lucee/runtime/ComponentImpl.java +++ b/core/src/main/java/lucee/runtime/ComponentImpl.java @@ -99,9 +99,8 @@ import lucee.runtime.type.Struct; import lucee.runtime.type.StructImpl; import lucee.runtime.type.UDF; +import lucee.runtime.type.BoundUDF; import lucee.runtime.type.UDFGSProperty; -import lucee.runtime.type.UDFGetterProperty; -import lucee.runtime.type.UDFSetterProperty; import lucee.runtime.type.UDFImpl; import lucee.runtime.type.UDFPlus; import lucee.runtime.type.UDFProperties; @@ -450,6 +449,18 @@ public static Map duplicateUTFMap(ComponentImpl src, ComponentImpl trg for (Entry e: srcMap.entrySet()) { udf = e.getValue(); + // LDEV-6298 v2: share the flyweight accessor across original + duplicate. + // Slow-path dispatch is now bound at extraction time via BoundUDF, so it no longer + // trusts srcComponent — sharing is safe. pageSource compare so chained duplicates match. + // LDEV-3335: null owner means a stateless class-level flyweight (pool entry) — share unconditionally. + if (udf instanceof UDFGSProperty) { + Component owner = udf.getOwnerComponent(); + if (owner == null || owner.getPageSource() == src.getPageSource()) { + trgMap.put(e.getKey(), udf); + } + continue; + } + if (udf.getOwnerComponent() == src) { UDF clone = e.getValue().duplicate(); if (clone instanceof UDFPlus) { @@ -770,14 +781,13 @@ else if (_namedArgs != null) { Object _call(PageContext pc, Collection.Key calledName, UDF udf, Struct namedArgs, Object[] args) throws PageException { - // LDEV-6236 accessor bypass — skip full UDF dispatch for generated getters/setters - if (!((PageContextImpl) pc).hasDebugOptions(ConfigPro.DEBUG_TEMPLATE)) { - if (udf instanceof UDFGetterProperty) { - return ((UDFGetterProperty) udf).callDirect( this, pc ); - } - if (udf instanceof UDFSetterProperty && args != null) { - return ((UDFSetterProperty) udf).callDirect( this, pc, args ); - } + // LDEV-6236 accessor bypass — skip full UDF dispatch for generated getters/setters. + // Guard both args paths: setter named-arg dispatch via UDFUtil.argumentCollection NPEs on null values. + if (!((PageContextImpl) pc).hasDebugOptions(ConfigPro.DEBUG_TEMPLATE) && udf instanceof UDFGSProperty) { + UDFGSProperty gs = (UDFGSProperty) udf; + if (args != null) return gs._call(pc, this, args); + if (namedArgs != null) return gs._callWithNamedValues(pc, this, namedArgs); + // both null — fall through to the slow path which has its own arg handling } Object rtn = null; @@ -1988,6 +1998,8 @@ private Object _set(PageContext pc, Collection.Key key, Object value, int access Member m = (Member) value; if (m instanceof UDF) { UDF udf = (UDF) m; + // LDEV-1962: unwrap BoundUDF on mixin assign — host component becomes the receiver. + if (udf instanceof BoundUDF) udf = ((BoundUDF) udf).getInner(); if (udf.getAccess() > Component.ACCESS_PUBLIC && udf instanceof UDFPlus) ((UDFPlus) udf).setAccess(Component.ACCESS_PUBLIC); _data.put(key, udf); _udfs.put(key, udf); @@ -2143,7 +2155,7 @@ public final Object put(Object key, Object value) { @Override public Object get(PageContext pc, Collection.Key key) throws PageException { Member member = getMember(pc, key, true, false); - if (member != null) return member.getValue(); + if (member != null) return accessorOrValue(member); // trigger if (triggerDataMember(pc) && !isPrivate(pc)) { @@ -2155,6 +2167,12 @@ public Object get(PageContext pc, Collection.Key key) throws PageException { // ["+name+"]"); } + // LDEV-6298 v2: bind shared accessor flyweight to this instance for slow-path extraction. + private Object accessorOrValue(Member member) { + if (member instanceof UDFGSProperty) return new BoundUDF((UDFGSProperty) member, this); + return member.getValue(); + } + private Object callGetter(PageContext pc, Collection.Key key) throws PageException { Key getterName = KeyImpl.init("get" + key.getLowerString()); Member member = getMember(pc, getterName, false, false); @@ -2211,7 +2229,7 @@ public Object get(int access, String name) throws PageException { @Override public Object get(int access, Collection.Key key) throws PageException { Member member = getMember(access, key, true, false); - if (member != null) return member.getValue(); + if (member != null) return accessorOrValue(member); // Trigger PageContext pc = ThreadLocalPageContext.get(); @@ -2224,7 +2242,7 @@ public Object get(int access, Collection.Key key) throws PageException { @Override public Object get(PageContext pc, Collection.Key key, Object defaultValue) { Member member = getMember(pc, key, true, false); - if (member != null) return member.getValue(); + if (member != null) return accessorOrValue(member); // trigger if (triggerDataMember(pc) && !isPrivate(pc)) { @@ -2253,7 +2271,7 @@ protected Object get(int access, String name, Object defaultValue) { @Override public Object get(int access, Collection.Key key, Object defaultValue) { Member member = getMember(access, key, true, false); - if (member != null) return member.getValue(); + if (member != null) return accessorOrValue(member); // trigger PageContext pc = ThreadLocalPageContext.get(); diff --git a/core/src/main/java/lucee/runtime/ComponentScopeShadow.java b/core/src/main/java/lucee/runtime/ComponentScopeShadow.java index b292cf61007..dbed6ffe910 100755 --- a/core/src/main/java/lucee/runtime/ComponentScopeShadow.java +++ b/core/src/main/java/lucee/runtime/ComponentScopeShadow.java @@ -35,6 +35,7 @@ import lucee.runtime.op.Duplicator; import lucee.runtime.type.Collection; import lucee.runtime.type.Struct; +import lucee.runtime.type.BoundUDF; import lucee.runtime.type.StructImpl; import lucee.runtime.type.UDF; import lucee.runtime.type.dt.DateTime; @@ -215,6 +216,9 @@ public Object removeEL(Key key) { public Object set(Collection.Key key, Object value) throws ApplicationException { if (key.equalsIgnoreCase(KeyConstants._this) || key.equalsIgnoreCase(KeyConstants._super) || key.equalsIgnoreCase(KeyConstants._static)) return value; + // LDEV-1962: mirror ComponentImpl._set — unwrap BoundUDF on assign so mixin rebind applies. + if (value instanceof BoundUDF) value = ((BoundUDF) value).getInner(); + if (!component.afterConstructor && value instanceof UDF) { component.addConstructorUDF(key, (UDF) value); } diff --git a/core/src/main/java/lucee/runtime/type/BoundUDF.java b/core/src/main/java/lucee/runtime/type/BoundUDF.java new file mode 100644 index 00000000000..0fe03cd1607 --- /dev/null +++ b/core/src/main/java/lucee/runtime/type/BoundUDF.java @@ -0,0 +1,280 @@ +package lucee.runtime.type; + +import lucee.runtime.Component; +import lucee.runtime.PageContext; +import lucee.runtime.PageContextImpl; +import lucee.runtime.PageSource; +import lucee.runtime.dump.DumpData; +import lucee.runtime.dump.DumpProperties; +import lucee.runtime.exp.PageException; +import lucee.runtime.type.Collection.Key; + +/** + * LDEV-6298 v2: bound-method wrapper around a shared {@link UDFGSProperty} flyweight. + * + *

Captures the calling component at extraction time so slow-path dispatch (UDF references, + * bracket-key lookup, higher-order pass) reads scope from the right instance even when the inner + * flyweight is shared across instances — both for {@code Duplicate(cfc)} (the LDEV-6298 v2 share) + * and for the class-level static accessor pool that LDEV-3335 emits but doesn't yet consume. Both + * threads converge on the same dispatch contract: {@code _call(pc, comp, args)} no longer trusts + * {@code srcComponent}, so {@code srcComponent} can be the original owner (this share) or + * {@code null} (LDEV-3335 Option 2 revival). + * + *

Allocated on every extraction through {@link lucee.runtime.ComponentImpl#get}; the fast path + * ({@code obj.method()}) goes through {@code ComponentImpl._call} which dispatches via + * {@link UDFGSProperty#_call} directly without allocating a wrapper. + */ +public final class BoundUDF implements UDFPlus { + + private static final long serialVersionUID = 1L; + + private final UDFGSProperty inner; + private final Component callingComp; + + public BoundUDF(UDFGSProperty inner, Component callingComp) { + this.inner = inner; + this.callingComp = callingComp; + } + + public UDFGSProperty getInner() { + return inner; + } + + // Wrapper-transparent equality. Two BoundUDFs with the same inner are equal regardless of + // callingComp; a BoundUDF equals its raw inner. Required for Component equality (StructSupport + // iterates accessor keys and compares values via UDF.equals — pre-WIP UDFGSProperty.equals + // was signature-based, so cross-instance Component compares were equal). Hibernate's + // HBMCreator.createFKColumnName depends on this contract (LDEV-6298 v2 testMany2Many). + @Override + public boolean equals(Object other) { + if (other == this) return true; + if (other instanceof BoundUDF) other = ((BoundUDF) other).inner; + return inner.equals(other); + } + + @Override + public int hashCode() { + return inner.hashCode(); + } + + public Component getCallingComponent() { + return callingComp; + } + + // === Dispatch — uses callingComp instead of inner.srcComponent === + + @Override + public Object call(PageContext pc, Object[] args, boolean doIncludePath) throws PageException { + PageContextImpl pci = (PageContextImpl) pc; + UDF parent = pci.getActiveUDF(); + pci.setActiveUDF(inner); + try { + return inner._call(pc, callingComp, args); + } + finally { + pci.setActiveUDF(parent); + } + } + + @Override + public Object callWithNamedValues(PageContext pc, Struct values, boolean doIncludePath) throws PageException { + PageContextImpl pci = (PageContextImpl) pc; + UDF parent = pci.getActiveUDF(); + pci.setActiveUDF(inner); + try { + return inner._callWithNamedValues(pc, callingComp, values); + } + finally { + pci.setActiveUDF(parent); + } + } + + @Override + public Object call(PageContext pc, Key calledName, Object[] args, boolean doIncludePath) throws PageException { + PageContextImpl pci = (PageContextImpl) pc; + UDF parent = pci.getActiveUDF(); + Key parentName = pci.getActiveUDFCalledName(); + pci.setActiveUDF(inner); + pci.setActiveUDFCalledName(calledName); + try { + return inner._call(pc, callingComp, args); + } + finally { + pci.setActiveUDF(parent); + pci.setActiveUDFCalledName(parentName); + } + } + + @Override + public Object callWithNamedValues(PageContext pc, Key calledName, Struct values, boolean doIncludePath) throws PageException { + PageContextImpl pci = (PageContextImpl) pc; + UDF parent = pci.getActiveUDF(); + Key parentName = pci.getActiveUDFCalledName(); + pci.setActiveUDF(inner); + pci.setActiveUDFCalledName(calledName); + try { + return inner._callWithNamedValues(pc, callingComp, values); + } + finally { + pci.setActiveUDF(parent); + pci.setActiveUDFCalledName(parentName); + } + } + + @Override + public Object implementation(PageContext pageContext) throws Throwable { + return inner.implementation(pageContext); + } + + // === Metadata — delegate to inner === + + @Override + public FunctionArgument[] getFunctionArguments() { + return inner.getFunctionArguments(); + } + + @Override + @Deprecated + public Object getDefaultValue(PageContext pc, int index) throws PageException { + return inner.getDefaultValue(pc, index); + } + + @Override + public Object getDefaultValue(PageContext pc, int index, Object defaultValue) throws PageException { + return inner.getDefaultValue(pc, index, defaultValue); + } + + @Override + public int getIndex() { + return inner.getIndex(); + } + + @Override + public String getFunctionName() { + return inner.getFunctionName(); + } + + @Override + public boolean getOutput() { + return inner.getOutput(); + } + + @Override + public int getReturnType() { + return inner.getReturnType(); + } + + @Override + public boolean getBufferOutput(PageContext pc) { + return inner.getBufferOutput(pc); + } + + @Override + @Deprecated + public int getReturnFormat() { + return inner.getReturnFormat(); + } + + @Override + public int getReturnFormat(int defaultFormat) { + return inner.getReturnFormat(defaultFormat); + } + + @Override + public Boolean getSecureJson() { + return inner.getSecureJson(); + } + + @Override + public Boolean getVerifyClient() { + return inner.getVerifyClient(); + } + + @Override + public String getReturnTypeAsString() { + return inner.getReturnTypeAsString(); + } + + @Override + public String getDescription() { + return inner.getDescription(); + } + + @Override + public String getDisplayName() { + return inner.getDisplayName(); + } + + @Override + public String getHint() { + return inner.getHint(); + } + + @Override + public String getSource() { + return inner.getSource(); + } + + @Override + public Struct getMetaData(PageContext pc) throws PageException { + return inner.getMetaData(pc); + } + + @Override + public UDF duplicate() { + return new BoundUDF(inner, callingComp); + } + + @Override + @Deprecated + public Component getOwnerComponent() { + return callingComp; + } + + @Override + public String id() { + return inner.id(); + } + + @Override + public PageSource getPageSource() { + return inner.getPageSource(); + } + + // === Member === + + @Override + public int getAccess() { + return inner.getAccess(); + } + + @Override + public Object getValue() { + return this; + } + + @Override + public int getModifier() { + return inner.getModifier(); + } + + // === UDFPlus === + + @Override + public void setOwnerComponent(Component component) { + // BoundUDF binding is captured at construction; the inner flyweight is shared so its + // owner must not be mutated through this path. setOwnerComponent on the wrapper is a no-op. + } + + @Override + public void setAccess(int access) { + inner.setAccess(access); + } + + // === Dumpable === + + @Override + public DumpData toDumpData(PageContext pageContext, int maxlevel, DumpProperties properties) { + return inner.toDumpData(pageContext, maxlevel, properties); + } +} diff --git a/core/src/main/java/lucee/runtime/type/UDFAddProperty.java b/core/src/main/java/lucee/runtime/type/UDFAddProperty.java index 5aabb205c42..7e270155e2f 100644 --- a/core/src/main/java/lucee/runtime/type/UDFAddProperty.java +++ b/core/src/main/java/lucee/runtime/type/UDFAddProperty.java @@ -65,30 +65,27 @@ public UDF duplicate() { } @Override - public Object _call(PageContext pageContext, Object[] args, boolean doIncludePath) throws PageException { - Component c = getComponent(pageContext); + public Object _call(PageContext pageContext, Component comp, Object[] args) throws PageException { // struct if (this.arguments.length == 2) { if (args.length < 2) throw new ExpressionException( "The function [" + getFunctionName() + "] needs 2 arguments, only " + args.length + " argument" + (args.length == 1 ? " is" : "s are") + " passed in."); - return _call(pageContext, c, args[0], args[1]); + return _call(pageContext, comp, args[0], args[1]); } // array else if (this.arguments.length == 1) { if (args.length < 1) throw new ExpressionException("The parameter [" + this.arguments[0].getName() + "] to function [" + getFunctionName() + "] is required but was not passed in."); - return _call(pageContext, c, null, args[0]); + return _call(pageContext, comp, null, args[0]); } // never reached - return c; - + return comp; } @Override - public Object _callWithNamedValues(PageContext pageContext, Struct values, boolean doIncludePath) throws PageException { + public Object _callWithNamedValues(PageContext pageContext, Component comp, Struct values) throws PageException { UDFUtil.argumentCollection(values, getFunctionArguments()); - Component c = getComponent(pageContext); // struct if (this.arguments.length == 2) { @@ -99,7 +96,7 @@ public Object _callWithNamedValues(PageContext pageContext, Struct values, boole if (key == null) throw new ExpressionException("The parameter [" + keyName + "] to function [" + getFunctionName() + "] is required but was not passed in."); if (value == null) throw new ExpressionException("The parameter [" + valueName + "] to function [" + getFunctionName() + "] is required but was not passed in."); - return _call(pageContext, c, key, value); + return _call(pageContext, comp, key, value); } // array else if (this.arguments.length == 1) { @@ -112,11 +109,11 @@ else if (this.arguments.length == 1) { } else throw new ExpressionException("The parameter [" + valueName + "] to function [" + getFunctionName() + "] is required but was not passed in."); } - return _call(pageContext, c, null, value); + return _call(pageContext, comp, null, value); } // never reached - return getComponent(pageContext); + return comp; } private Object _call(PageContext pageContext, Component c, Object key, Object value) throws PageException { diff --git a/core/src/main/java/lucee/runtime/type/UDFGSProperty.java b/core/src/main/java/lucee/runtime/type/UDFGSProperty.java index dc332defdc0..30cd9229e1e 100755 --- a/core/src/main/java/lucee/runtime/type/UDFGSProperty.java +++ b/core/src/main/java/lucee/runtime/type/UDFGSProperty.java @@ -311,29 +311,29 @@ public final Object call(PageContext pageContext, Object[] args, boolean doInclu UDF parent = pci.getActiveUDF(); pci.setActiveUDF(this); try { - return _call(pageContext, args, doIncludePath); + return _call(pageContext, getComponent(pageContext), args); } finally { pci.setActiveUDF(parent); } } - public abstract Object _call(PageContext pageContext, Object[] args, boolean doIncludePath) throws PageException; - @Override public final Object callWithNamedValues(PageContext pageContext, Struct values, boolean doIncludePath) throws PageException { PageContextImpl pci = (PageContextImpl) pageContext; UDF parent = pci.getActiveUDF(); pci.setActiveUDF(this); try { - return _callWithNamedValues(pageContext, values, doIncludePath); + return _callWithNamedValues(pageContext, getComponent(pageContext), values); } finally { pci.setActiveUDF(parent); } } - public abstract Object _callWithNamedValues(PageContext pageContext, Struct values, boolean doIncludePath) throws PageException; + public abstract Object _call(PageContext pageContext, Component comp, Object[] args) throws PageException; + + public abstract Object _callWithNamedValues(PageContext pageContext, Component comp, Struct values) throws PageException; private static String createMessage(String format, Object value) { if (Decision.isSimpleValue(value)) return "the value [" + Caster.toString(value, null) + "] is not in [" + format + "] format"; diff --git a/core/src/main/java/lucee/runtime/type/UDFGetterProperty.java b/core/src/main/java/lucee/runtime/type/UDFGetterProperty.java index 7c085af871d..51c36e06241 100755 --- a/core/src/main/java/lucee/runtime/type/UDFGetterProperty.java +++ b/core/src/main/java/lucee/runtime/type/UDFGetterProperty.java @@ -20,7 +20,6 @@ import lucee.commons.lang.CFTypes; import lucee.runtime.Component; -import lucee.runtime.ComponentImpl; import lucee.runtime.PageContext; import lucee.runtime.component.Property; import lucee.runtime.component.PropertyImpl; @@ -47,22 +46,14 @@ public UDF duplicate() { return new UDFGetterProperty(srcComponent, prop); } - /** - * Direct accessor bypass — called from ComponentImpl._call() to skip UDF dispatch overhead. - * The caller already knows the component, so we skip getComponent(pc) resolution. - */ - public Object callDirect( ComponentImpl comp, PageContext pc ) { - return comp.getComponentScope().get( pc, propName, null ); - } - @Override - public Object _call(PageContext pageContext, Object[] args, boolean doIncludePath) throws PageException { - return getComponent(pageContext).getComponentScope().get(pageContext, propName, null); + public Object _call(PageContext pageContext, Component comp, Object[] args) throws PageException { + return comp.getComponentScope().get(pageContext, propName, null); } @Override - public Object _callWithNamedValues(PageContext pageContext, Struct values, boolean doIncludePath) throws PageException { - return getComponent(pageContext).getComponentScope().get(pageContext, propName, null); + public Object _callWithNamedValues(PageContext pageContext, Component comp, Struct values) throws PageException { + return comp.getComponentScope().get(pageContext, propName, null); } @Override diff --git a/core/src/main/java/lucee/runtime/type/UDFHasProperty.java b/core/src/main/java/lucee/runtime/type/UDFHasProperty.java index 862e9fbe773..03127370910 100644 --- a/core/src/main/java/lucee/runtime/type/UDFHasProperty.java +++ b/core/src/main/java/lucee/runtime/type/UDFHasProperty.java @@ -73,13 +73,13 @@ public UDF duplicate() { } @Override - public Object _call(PageContext pageContext, Object[] args, boolean doIncludePath) throws PageException { - if (args.length < 1) return has(pageContext); - return has(pageContext, args[0]); + public Object _call(PageContext pageContext, Component comp, Object[] args) throws PageException { + if (args.length < 1) return has(comp); + return has(comp, args[0]); } @Override - public Object _callWithNamedValues(PageContext pageContext, Struct values, boolean doIncludePath) throws PageException { + public Object _callWithNamedValues(PageContext pageContext, Component comp, Struct values) throws PageException { UDFUtil.argumentCollection(values, getFunctionArguments()); Key key = arguments[0].getName(); Object value = values.get(key, null); @@ -88,14 +88,14 @@ public Object _callWithNamedValues(PageContext pageContext, Struct values, boole if (keys.length > 0) { value = values.get(keys[0]); } - else return has(pageContext); + else return has(comp); } - return has(pageContext, value); + return has(comp, value); } - private boolean has(PageContext pageContext) { - Object propValue = getComponent(pageContext).getComponentScope().get(propName, null); + private boolean has(Component comp) { + Object propValue = comp.getComponentScope().get(propName, null); // struct if (isStruct()) { @@ -118,8 +118,8 @@ else if (propValue instanceof java.util.List) { } - private boolean has(PageContext pageContext, Object value) throws PageException { - Object propValue = getComponent(pageContext).getComponentScope().get(propName, null); + private boolean has(Component comp, Object value) throws PageException { + Object propValue = comp.getComponentScope().get(propName, null); // struct if (isStruct()) { diff --git a/core/src/main/java/lucee/runtime/type/UDFRemoveProperty.java b/core/src/main/java/lucee/runtime/type/UDFRemoveProperty.java index fdd515ae5b9..4c12b281ac1 100644 --- a/core/src/main/java/lucee/runtime/type/UDFRemoveProperty.java +++ b/core/src/main/java/lucee/runtime/type/UDFRemoveProperty.java @@ -73,15 +73,15 @@ public UDF duplicate() { } @Override - public Object _call(PageContext pageContext, Object[] args, boolean doIncludePath) throws PageException { + public Object _call(PageContext pageContext, Component comp, Object[] args) throws PageException { if (args.length < 1) throw new ExpressionException("The parameter [" + this.arguments[0].getName() + "] to function [" + getFunctionName() + "] is required but was not passed in."); - return remove(pageContext, args[0]); + return remove(comp, pageContext, args[0]); } @Override - public Object _callWithNamedValues(PageContext pageContext, Struct values, boolean doIncludePath) throws PageException { + public Object _callWithNamedValues(PageContext pageContext, Component comp, Struct values) throws PageException { UDFUtil.argumentCollection(values, getFunctionArguments()); Key key = arguments[0].getName(); Object value = values.get(key, null); @@ -93,11 +93,10 @@ public Object _callWithNamedValues(PageContext pageContext, Struct values, boole else throw new ExpressionException("The parameter [" + key + "] to function [" + getFunctionName() + "] is required but was not passed in."); } - return remove(pageContext, value); + return remove(comp, pageContext, value); } - private boolean remove(PageContext pageContext, Object value) throws PageException { - Component c = getComponent(pageContext); + private boolean remove(Component c, PageContext pageContext, Object value) throws PageException { Object propValue = c.getComponentScope().get(propName, null); value = cast(pageContext, arguments[0], value, 1); diff --git a/core/src/main/java/lucee/runtime/type/UDFSetterProperty.java b/core/src/main/java/lucee/runtime/type/UDFSetterProperty.java index c6f6814bf81..f448b49ee34 100755 --- a/core/src/main/java/lucee/runtime/type/UDFSetterProperty.java +++ b/core/src/main/java/lucee/runtime/type/UDFSetterProperty.java @@ -21,7 +21,6 @@ import lucee.commons.lang.CFTypes; import lucee.commons.lang.StringUtil; import lucee.runtime.Component; -import lucee.runtime.ComponentImpl; import lucee.runtime.PageContext; import lucee.runtime.component.Property; import lucee.runtime.component.PropertyImpl; @@ -90,42 +89,23 @@ public UDF duplicate() { return new UDFSetterProperty(srcComponent, prop, validate, validateParams); } - /** - * Direct accessor bypass — called from ComponentImpl._call() to skip UDF dispatch overhead. - * The caller already knows the component, so we skip getComponent(pc) resolution. - */ - public Object callDirect( ComponentImpl comp, PageContext pc, Object[] args ) throws PageException { - if (args == null || args.length < 1) - throw new ExpressionException( "The parameter " + prop.getName() + " to function " + getFunctionName() + " is required but was not passed in." ); - validate( validate, validateParams, args[0] ); - comp.getComponentScope().set( propName, cast( pc, this.arguments[0], args[0], 1 ) ); - - ApplicationContext appContext = pc.getApplicationContext(); - if (appContext.isORMEnabled() && comp.isPersistent()) ORMUtil.getSession( pc ); - - return comp; - } - @Override - public Object _call(PageContext pageContext, Object[] args, boolean doIncludePath) throws PageException { - if (args.length < 1) throw new ExpressionException("The parameter " + prop.getName() + " to function " + getFunctionName() + " is required but was not passed in."); + public Object _call(PageContext pageContext, Component comp, Object[] args) throws PageException { + if (args == null || args.length < 1) + throw new ExpressionException("The parameter " + prop.getName() + " to function " + getFunctionName() + " is required but was not passed in."); validate(validate, validateParams, args[0]); - Component c = getComponent(pageContext); - c.getComponentScope().set(propName, cast(pageContext, this.arguments[0], args[0], 1)); + comp.getComponentScope().set(propName, cast(pageContext, this.arguments[0], args[0], 1)); - // make sure it is reconized that set is called by hibernate - // if(component.isPersistent())ORMUtil.getSession(pageContext); ApplicationContext appContext = pageContext.getApplicationContext(); - if (appContext.isORMEnabled() && c.isPersistent()) ORMUtil.getSession(pageContext); + if (appContext.isORMEnabled() && comp.isPersistent()) ORMUtil.getSession(pageContext); - return c; + return comp; } @Override - public Object _callWithNamedValues(PageContext pageContext, Struct values, boolean doIncludePath) throws PageException { + public Object _callWithNamedValues(PageContext pageContext, Component comp, Struct values) throws PageException { UDFUtil.argumentCollection(values, getFunctionArguments()); Object value = values.get(propName, null); - Component c = getComponent(pageContext); if (value == null) { Key[] keys = CollectionUtil.keys(values); @@ -134,14 +114,12 @@ public Object _callWithNamedValues(PageContext pageContext, Struct values, boole } else throw new ExpressionException("The parameter " + prop.getName() + " to function " + getFunctionName() + " is required but was not passed in."); } - c.getComponentScope().set(propName, cast(pageContext, arguments[0], value, 1)); + comp.getComponentScope().set(propName, cast(pageContext, arguments[0], value, 1)); - // make sure it is reconized that set is called by hibernate - // if(component.isPersistent())ORMUtil.getSession(pageContext); ApplicationContext appContext = pageContext.getApplicationContext(); - if (appContext.isORMEnabled() && c.isPersistent()) ORMUtil.getSession(pageContext); + if (appContext.isORMEnabled() && comp.isPersistent()) ORMUtil.getSession(pageContext); - return c; + return comp; } @Override diff --git a/test/general/Accessors.cfc b/test/general/Accessors.cfc index a5dfb269a35..379907e5c61 100644 --- a/test/general/Accessors.cfc +++ b/test/general/Accessors.cfc @@ -377,12 +377,11 @@ component extends="org.lucee.cfml.test.LuceeTestCase" { foo.setA( "mutated-after-dup" ); expect( refDup() ).toBe( "mutated-after-dup" ); }); - xit( title="re-extracting an injected accessor rebinds again — confirms unwrap on assignment", body=function( currentSpec ){ - // SKIPPED — known-failing on baseline. The slow-path extraction on 7.0/baseline-7.1 - // returns the raw UDFGetterProperty with srcComponent fallback (reads from foo); - // the rebind contract only kicks in via the BoundUDF wrap on extraction. Once - // BoundUDF lands in trunk, flip back to it() and this asserts the fast/slow - // dispatch paths agree on receiver = host (bar). + it( title="re-extracting an injected accessor rebinds again — confirms unwrap on assignment", body=function( currentSpec ){ + // LDEV-6298 v2: the BoundUDF wrap on extraction is what makes this pass — the + // slow-path extraction now returns a wrapper bound to the host (bar), so fast/slow + // dispatch paths agree on receiver. Pre-v2 (7.0/baseline-7.1) this fails because + // the raw UDFGetterProperty falls back to srcComponent and reads from foo. var foo = new accessors.testWithAccessors(); var bar = new accessors.testWithAccessors(); foo.setA( "from-foo" ); @@ -393,6 +392,35 @@ component extends="org.lucee.cfml.test.LuceeTestCase" { }); }); + describe( "BoundUDF wrapper transparency under equality", function(){ + // LDEV-6298 v2: BoundUDF wraps every slow-path extraction (one fresh wrapper per + // `obj.method` read). For Component.equals to keep behaving like baseline, the wrapper + // must compare equal to other wrappers around the same inner, and to the raw inner + // UDFGSProperty itself. Hibernate's HBMCreator.createFKColumnName depends on this: + // `_cfc.equals(cfc)` walks the component's data slots and compares accessor UDFs by + // signature — without wrapper-transparent equals, FK column resolution breaks for + // every one-to-many / many-to-one relationship. Pre-v2 contract: UDFGSProperty.equals + // is signature-based (UDFImpl.equals). + it( title="two fresh instances of the same CFC compare equal — drives Hibernate FK resolution", body=function( currentSpec ){ + var a = new accessors.testWithAccessors(); + var b = new accessors.testWithAccessors(); + expect( ObjectEquals( a, b ) ).toBeTrue(); + }); + it( title="two refs to the same accessor on different instances compare equal", body=function( currentSpec ){ + var a = new accessors.testWithAccessors(); + var b = new accessors.testWithAccessors(); + var refA = a.getA; + var refB = b.getA; + expect( ObjectEquals( refA, refB ) ).toBeTrue(); + }); + it( title="two refs from the SAME instance — repeated extraction yields equal wrappers", body=function( currentSpec ){ + var foo = new accessors.testWithAccessors(); + var ref1 = foo.getA; + var ref2 = foo.getA; + expect( ObjectEquals( ref1, ref2 ) ).toBeTrue(); + }); + }); + describe( "storage scope — variables vs this", function(){ // per the component-accessors recipe: defaults seed variables only; accessors read/write // variables; this is a separate (public) scope and is not populated automatically. From f6cd3cdb1fe8a948b4d65e5157f88a2e6e4d41a0 Mon Sep 17 00:00:00 2001 From: Zac Spitzer Date: Sun, 3 May 2026 01:24:30 +0200 Subject: [PATCH 2/2] LDEV-6300 tests for LDEV-6298 v2 BoundUDF share invariants via Java reflection --- test/tickets/LDEV6300.cfc | 108 ++++++++++++++++++++++ test/tickets/LDEV6300/BasePerson.cfc | 4 + test/tickets/LDEV6300/InheritedPerson.cfc | 3 + test/tickets/LDEV6300/Person.cfc | 4 + 4 files changed, 119 insertions(+) create mode 100644 test/tickets/LDEV6300.cfc create mode 100644 test/tickets/LDEV6300/BasePerson.cfc create mode 100644 test/tickets/LDEV6300/InheritedPerson.cfc create mode 100644 test/tickets/LDEV6300/Person.cfc diff --git a/test/tickets/LDEV6300.cfc b/test/tickets/LDEV6300.cfc new file mode 100644 index 00000000000..eeb2928d25d --- /dev/null +++ b/test/tickets/LDEV6300.cfc @@ -0,0 +1,108 @@ +component extends="org.lucee.cfml.test.LuceeTestCase" labels="java,component" { + + // LDEV-6300 — structural invariants underpinning the LDEV-6298 v2 flyweight share. + // + // CFML can't observe these contracts directly. A violation breaks the share gate at + // duplicateUTFMap / addUDFS without breaking dispatch, so functional bedrock under + // test/general/Accessors.cfc wouldn't catch it. The original investigation + // (setOwner-corrupts-shared-UDFGSProperty) used Java-reflection tripwire scripts; this + // file ports the durable invariant probes into TestBox so future refactors that touch + // UDFGSProperty / ComponentImpl write paths land against red tests, not silent perf + // regressions in tests-orm bench runs. + + function run( testResults, testBox ) { + + describe( "LDEV-6300 / LDEV-6298 v2 — flyweight share invariants", function(){ + + // Locks in current behaviour: PropertyFactory.createGetter allocates fresh UDFGetterProperty + // per init via setProperty (LDEV-3335 static pool emitted but unconsumed; LDEV-6236 no-owner + // share branch in addUDFS doesn't fire). If LDEV-3335 Option 2 ever revives the pool, this + // spec flips and the assertion gets inverted. + it( title="fresh same-class siblings get distinct UDFGSProperty Java instances — current contract, no static-pool consumption", body=function( currentSpec ){ + var A = new LDEV6300.Person(); + var B = new LDEV6300.Person(); + expect( idOf( probeUdf( A, "getName" ) ) ).notToBe( idOf( probeUdf( B, "getName" ) ) ); + }); + + // Same locked behaviour for explicit-extends instantiation: ComponentLoader.searchComponent + // allocates a fresh ComponentImpl for the base on every call, with its own PropertyFactory'd + // accessors. LDEV-6300 phase 3 (share-base) is what would change this. + it( title="explicit-extends subclass and bare base get distinct UDFGSProperty Java instances — current contract, no base-instance reuse", body=function( currentSpec ){ + var base = new LDEV6300.BasePerson(); + var sub = new LDEV6300.InheritedPerson(); + expect( idOf( probeUdf( base, "getName" ) ) ).notToBe( idOf( probeUdf( sub, "getName" ) ) ); + // child's own accessor still works — sanity check the fixture + sub.setRole( "admin" ); + expect( sub.getRole() ).toBe( "admin" ); + }); + + it( title="Duplicate(cfc) shares the UDFGSProperty Java instance with source — LDEV-6298 v2 contract", body=function( currentSpec ){ + var A = new LDEV6300.Person(); + A.setName( "alpha" ); + var D = duplicate( A ); + expect( idOf( probeUdf( A, "getName" ) ) ).toBe( idOf( probeUdf( D, "getName" ) ) ); + expect( D.getName() ).toBe( "alpha" ); + }); + + it( title="shared flyweight srcComponent stable across sibling instantiation — fresh new() must not mutate the share", body=function( currentSpec ){ + var A = new LDEV6300.Person(); + var srcBefore = idOf( srcOf( probeUdf( A, "getName" ) ) ); + var B = new LDEV6300.Person(); + expect( idOf( srcOf( probeUdf( A, "getName" ) ) ) ).toBe( srcBefore ); + }); + + it( title="shared flyweight srcComponent stable across Duplicate(cfc) — duplicate path must not mutate the share", body=function( currentSpec ){ + var A = new LDEV6300.Person(); + A.setName( "alpha" ); + var srcBefore = idOf( srcOf( probeUdf( A, "getName" ) ) ); + var D = duplicate( A ); + expect( idOf( srcOf( probeUdf( A, "getName" ) ) ) ).toBe( srcBefore ); + }); + + it( title="shared flyweight srcComponent stable across sibling ObjectSave/ObjectLoad — original LDEV-6298 investigation contract", body=function( currentSpec ){ + var A = new LDEV6300.Person(); + A.setName( "alpha" ); + var B = new LDEV6300.Person(); + B.setName( "bravo" ); + var srcA = idOf( srcOf( probeUdf( A, "getName" ) ) ); + var srcB = idOf( srcOf( probeUdf( B, "getName" ) ) ); + + var C = new LDEV6300.Person(); + C.setName( "charlie" ); + var D = ObjectLoad( ObjectSave( C ) ); + + expect( idOf( srcOf( probeUdf( A, "getName" ) ) ) ).toBe( srcA ); + expect( idOf( srcOf( probeUdf( B, "getName" ) ) ) ).toBe( srcB ); + expect( D.getName() ).toBe( "charlie" ); + expect( A.getName() ).toBe( "alpha" ); + expect( B.getName() ).toBe( "bravo" ); + }); + + }); + } + + // === Java-reflection helpers === + + // _udfs is a private field on ComponentImpl; srcComponent is a private field declared on + // UDFGSProperty. Both are needed to assert structural invariants that have no CFML surface. + + private any function probeUdf( required any cfc, required string key ) { + var clazz = createObject( "java", "java.lang.Class" ).forName( "lucee.runtime.ComponentImpl" ); + var udfsField = clazz.getDeclaredField( "_udfs" ); + udfsField.setAccessible( true ); + var keyImpl = createObject( "java", "lucee.runtime.type.KeyImpl" ).init( arguments.key ); + return udfsField.get( arguments.cfc ).get( keyImpl ); + } + + private any function srcOf( required any udf ) { + var clazz = createObject( "java", "java.lang.Class" ).forName( "lucee.runtime.type.UDFGSProperty" ); + var srcField = clazz.getDeclaredField( "srcComponent" ); + srcField.setAccessible( true ); + return srcField.get( arguments.udf ); + } + + private numeric function idOf( required any obj ) { + return createObject( "java", "java.lang.System" ).identityHashCode( arguments.obj ); + } + +} diff --git a/test/tickets/LDEV6300/BasePerson.cfc b/test/tickets/LDEV6300/BasePerson.cfc new file mode 100644 index 00000000000..5c220be36dd --- /dev/null +++ b/test/tickets/LDEV6300/BasePerson.cfc @@ -0,0 +1,4 @@ +component accessors="true" { + property name="name" type="string"; + property name="email" type="string"; +} diff --git a/test/tickets/LDEV6300/InheritedPerson.cfc b/test/tickets/LDEV6300/InheritedPerson.cfc new file mode 100644 index 00000000000..41ce71a314b --- /dev/null +++ b/test/tickets/LDEV6300/InheritedPerson.cfc @@ -0,0 +1,3 @@ +component extends="BasePerson" accessors="true" { + property name="role" type="string"; +} diff --git a/test/tickets/LDEV6300/Person.cfc b/test/tickets/LDEV6300/Person.cfc new file mode 100644 index 00000000000..5c220be36dd --- /dev/null +++ b/test/tickets/LDEV6300/Person.cfc @@ -0,0 +1,4 @@ +component accessors="true" { + property name="name" type="string"; + property name="email" type="string"; +}