daniellansun commented on code in PR #2736:
URL: https://github.com/apache/groovy/pull/2736#discussion_r3657338121
##########
src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java:
##########
@@ -215,29 +216,86 @@ public int getOrderNumber() {
}
/**
- * Shared switch point invalidated when metaclass state changes.
+ * Legacy process-wide SwitchPoint retained for binary compatibility only.
+ * <p>
+ * <strong>Behavioral change in 6.0 (GROOVY-12191):</strong> MetaClass
changes
+ * are scoped per {@link org.codehaus.groovy.reflection.ClassInfo}. This
field
+ * is <em>not</em> the MOP guard on linked call sites and is <em>not</em>
+ * rotated on per-class MetaClass changes. It is rotated only when
+ * {@link #invalidateSwitchPoints()} runs — i.e. category enter/leave and
+ * {@code VMPlugin.invalidateCallSites()} — so external observers of this
+ * field still see those bulk events. Guarding a site on this field alone
+ * will <strong>silently miss</strong> type-scoped MetaClass invalidations;
+ * migrate to {@link IndyInvalidation#guardWithMopSwitchPoints}.
+ *
+ * @see IndyInvalidation
+ * @deprecated since 6.0.0 — use {@link
IndyInvalidation#guardWithMopSwitchPoints};
+ * this field is not the call-site MOP guard and is not
rotated on
+ * per-class MetaClass changes.
*/
- protected static SwitchPoint switchPoint = new SwitchPoint();
+ @Deprecated(since = "6.0.0", forRemoval = false)
+ protected static volatile SwitchPoint switchPoint = new SwitchPoint();
static {
-
GroovySystem.getMetaClassRegistry().addMetaClassRegistryChangeEventListener(cmcu
-> invalidateSwitchPoints());
+ // MetaClass registry changes invalidate the affected class + subtypes
(GROOVY-12191).
+ // Hierarchy fan-out is owned here (not duplicated in
setStrongMetaClass beyond a
+ // local retire of the exact class for paths that never fire the
registry).
+
GroovySystem.getMetaClassRegistry().addMetaClassRegistryChangeEventListener(cmcu
-> {
+ Class<?> type = cmcu.getClassToUpdate();
+ if (type != null) {
+ IndyInvalidation.invalidateClass(type);
+ if (LOG_ENABLED) {
+ LOG.info("invalidating class SwitchPoint hierarchy for " +
type.getName());
+ }
+ } else {
+ IndyInvalidation.invalidateUnscoped();
+ if (LOG_ENABLED) {
+ LOG.info("unscoped SwitchPoint invalidation (unattributed
MetaClass change)");
+ }
+ }
+ });
}
/**
- * Callback for constant metaclass update change
+ * Category enter/leave and {@code VMPlugin.invalidateCallSites()}.
+ * Bulk-invalidates every loaded class SwitchPoint so sites re-link under
the
+ * new category state, and rotates the legacy {@link #switchPoint} field
under
+ * {@code synchronized (IndyInterface.class)} so concurrent bulk
invalidations
+ * cannot orphan a live SwitchPoint an external reader just observed.
+ * Per-class MetaClass changes use {@link
IndyInvalidation#invalidateClass(Class)}
+ * (or {@link org.codehaus.groovy.reflection.ClassInfo#incVersion()})
instead
+ * and do <em>not</em> rotate {@link #switchPoint}.
*/
protected static void invalidateSwitchPoints() {
if (LOG_ENABLED) {
- LOG.info("invalidating switch point");
+ LOG.info("invalidating class SwitchPoints for category /
invalidateCallSites");
}
-
+ IndyInvalidation.invalidateCategory();
+ // Binary-compat: rotate the legacy field so external observers of bulk
+ // invalidation still see a change. Synchronized so two concurrent
callers
+ // cannot leave a reader holding a live SwitchPoint that will never be
+ // invalidated again (pre-6.0 shape; addresses Sonar S3077 /
GROOVY-12191).
synchronized (IndyInterface.class) {
SwitchPoint old = switchPoint;
switchPoint = new SwitchPoint();
- SwitchPoint.invalidateAll(new SwitchPoint[]{old});
+ if (old != null && !old.hasBeenInvalidated()) {
+ SwitchPoint.invalidateAll(new SwitchPoint[]{old});
+ }
}
}
+ /**
+ * Installs the per-class MOP SwitchPoint guard on a linked handle.
+ *
+ * @param handle fast-path handle
+ * @param fallback re-link handle
+ * @param receiver call receiver (may be {@code null})
+ * @return guarded handle
+ */
+ static MethodHandle applyMopSwitchPoints(final MethodHandle handle, final
MethodHandle fallback, final Object receiver) {
+ return IndyInvalidation.guardWithMopSwitchPoints(handle, fallback,
receiver);
+ }
Review Comment:
Good question — the split is intentional, and the previous one-line delegate
made it look accidental.
**Rationale**
| Concern | Owner |
|--------|--------|
| Domain resolution (`receiver` → class → `ClassInfo` SwitchPoint) |
`IndyInvalidation` (also used by `ClassInfo`, cold tier, tests) |
| Invalidation policy (class / hierarchy / category / unscoped) |
`IndyInvalidation` |
| Link-time `guardWithTest` install on production call sites |
**`IndyInterface.applyMopSwitchPoints`** (`Selector`, `IndyCompoundAssign`) |
Putting invalidation policy next to bootstrap wiring would grow
`IndyInterface` further and pull `ClassInfo` toward `vmplugin`. Putting *only*
the bind step in `IndyInterface` keeps the bytecode surface as the place that
wires handles, while the runtime.indy package owns domains and retire rules.
**Action taken**
- `applyMopSwitchPoints` now performs the bind itself:
`IndyInvalidation.classSwitchPointFor(receiver).guardWithTest(handle,
fallback)`
- `IndyInvalidation.guardWithMopSwitchPoints` remains the **public / test**
entry (same semantics), documented as such.
- Javadoc on both sides spells out the division of responsibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
