From 61b0222f2dcfe3565cbb874e17c7b504586f2613 Mon Sep 17 00:00:00 2001 From: Patrick Reinhart Date: Tue, 30 Apr 2024 20:24:14 +0200 Subject: [PATCH 1/8] Use record for trigger state --- .../triggers/TriggerStatePerThread.java | 523 +++++++++--------- 1 file changed, 249 insertions(+), 274 deletions(-) diff --git a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java index b45d50e0696..eb69ab174dc 100644 --- a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java +++ b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java @@ -27,6 +27,7 @@ import java.util.ArrayDeque; import java.util.Deque; import java.util.Iterator; +import java.util.Objects; /** * Avoid infinite recursions in Triggers by preventing the same trigger @@ -35,278 +36,252 @@ * @author Adam Retter */ public class TriggerStatePerThread { - - private final static ThreadLocal> THREAD_LOCAL_STATES = ThreadLocal.withInitial(ArrayDeque::new); - - public static void setAndTest(final Trigger trigger, final TriggerPhase triggerPhase, final TriggerEvent triggerEvent, final XmldbURI src, final @Nullable XmldbURI dst) throws CyclicTriggerException { - final Deque states = THREAD_LOCAL_STATES.get(); - - if (states.isEmpty()) { - if (triggerPhase != TriggerPhase.BEFORE) { - throw new IllegalStateException("The Before phase of a trigger must occur before the After phase"); - } - states.addFirst(new TriggerState(trigger, triggerPhase, triggerEvent, src, dst)); - return; - } - - TriggerState prevState = states.peekFirst(); - - // is the new state the same as the previous state (excluding the phase) - if (prevState.equalsIgnoringPhase(trigger, triggerEvent, src, dst)) { - - // is this the after phase (i.e. matching completion) of a previous non-cyclic before phase? - if (triggerPhase == TriggerPhase.AFTER) { - - int skipBefores = 0; - - for (final Iterator it = states.iterator(); it.hasNext(); ) { - prevState = it.next(); - - // travel up, first "Before" we encounter - we should check if (a) that we complete it, and/or (b) is non-cyclic (if not we are also cyclic) - if (prevState.triggerPhase == TriggerPhase.BEFORE) { - - if (skipBefores > 0) { - skipBefores--; - - } else { - if (prevState.isCompletedBy(trigger, triggerPhase, triggerEvent, src, dst)) { - if (prevState instanceof PossibleCyclicTriggerState) { - // if the Before phase is a PossibleCyclicTriggerState then this completing After phase must also be a PossibleCyclicTriggerState - final TriggerState newState = new PossibleCyclicTriggerState(trigger, triggerPhase, triggerEvent, src, dst); - states.addFirst(newState); - - throw new CyclicTriggerException("Detected Matching possible cyclic trigger event for After phase (" + newState + ") of previous Before phase (" + prevState + ")"); - - } else { - // if the Before Phase is NOT a PossibleCyclicTriggerState, then neither is this completing After phase... - states.addFirst(new TriggerState(trigger, triggerPhase, triggerEvent, src, dst)); - return; - } - - } else { - throw new IllegalStateException("Cannot interleave Trigger states"); - } - } - } else if (prevState.triggerPhase == TriggerPhase.AFTER) { - skipBefores++; - } - } - - throw new IllegalStateException("Could not find a matching Before phase for After phase"); - - } else { - // it's a cyclic exception! - final TriggerState newState = new PossibleCyclicTriggerState(trigger, triggerPhase, triggerEvent, src, dst); - states.addFirst(newState); - - throw new CyclicTriggerException("Detected possible cyclic trigger events: " + newState); - } - } - - states.addFirst(new TriggerState(trigger, triggerPhase, triggerEvent, src, dst)); - } - - public static class CyclicTriggerException extends Exception { - public CyclicTriggerException(final String message) { - super(message); - } - } - - public static void clearIfFinished(final TriggerPhase phase) { - if (phase == TriggerPhase.AFTER) { - - int depth = 0; - final Deque states = THREAD_LOCAL_STATES.get(); - for (final Iterator it = states.descendingIterator(); it.hasNext(); ) { - final TriggerState state = it.next(); - switch (state.triggerPhase) { - case BEFORE: - depth++; - break; - case AFTER: - depth--; - break; - default: - throw new IllegalStateException("Unknown phase: " + state.triggerPhase + "for trigger state: " + state); - } - } - - if (depth == 0) { - clear(); - } - } - } - - public static void clear() { - THREAD_LOCAL_STATES.remove(); - } - - public static boolean isEmpty() { - return THREAD_LOCAL_STATES.get().isEmpty(); - } - - private static class PossibleCyclicTriggerState extends TriggerState { - public PossibleCyclicTriggerState(final TriggerState triggerState) { - super(triggerState.trigger, triggerState.triggerPhase, triggerState.triggerEvent, triggerState.src, triggerState.dst); - } - - public PossibleCyclicTriggerState(final Trigger trigger, final TriggerPhase triggerPhase, final TriggerEvent triggerEvent, final XmldbURI src, final @Nullable XmldbURI dst) { - super(trigger, triggerPhase, triggerEvent, src, dst); - } - } - - private static class TriggerState { - private final Trigger trigger; - private final TriggerPhase triggerPhase; - private final TriggerEvent triggerEvent; - private final XmldbURI src; - private final @Nullable XmldbURI dst; - - public TriggerState(final Trigger trigger, final TriggerPhase triggerPhase, final TriggerEvent triggerEvent, final XmldbURI src, final @Nullable XmldbURI dst) { - this.trigger = trigger; - this.triggerPhase = triggerPhase; - this.triggerEvent = triggerEvent; - this.src = src; - this.dst = dst; - } - - @Override - public String toString() { - final StringBuilder builder = new StringBuilder(); - builder.append(triggerPhase); - builder.append(' '); - builder.append(triggerEvent); - builder.append('('); - if (triggerPhase == TriggerPhase.AFTER && dst != null) { - builder.append(dst); - builder.append(", "); - } - builder.append(src); - if (triggerPhase == TriggerPhase.BEFORE && dst != null) { - builder.append(", "); - builder.append(dst); - } - builder.append(')'); - builder.append(": "); - builder.append(trigger.getClass().getSimpleName()); - if (trigger instanceof XQueryTrigger) { - final String urlQuery = ((XQueryTrigger) trigger).getUrlQuery(); - if (urlQuery != null && !urlQuery.isEmpty()) { - builder.append('('); - builder.append(urlQuery); - builder.append(')'); - } - } - return builder.toString(); - } - - @Override - public boolean equals(final Object o) { - return equals(o, false); - } - - public boolean equalsIgnoringPhase(final Object o) { - return equals(o, true); - } - - private boolean equals(final Object o, final boolean ignorePhase) { - if (this == o) { - return true; - } - - if (o == null || getClass() != o.getClass()) { - return false; - } - - final TriggerState that = (TriggerState) o; - - if (!trigger.equals(that.trigger)) { - return false; - } - - if (!ignorePhase) { - if (triggerPhase != that.triggerPhase) { - return false; - } - } - - if (triggerEvent != that.triggerEvent) { - return false; - } - - if (!src.equals(that.src)) { - return false; - } - - return dst != null ? dst.equals(that.dst) : that.dst == null; - } - - private boolean equalsIgnoringPhase(final Trigger otherTrigger, final TriggerEvent otherTriggerEvent, final XmldbURI otherSrc, @Nullable final XmldbURI otherDst) { - if (!trigger.equals(otherTrigger)) { - return false; - } - - if (triggerEvent != otherTriggerEvent) { - return false; - } - - if (!src.equals(otherSrc)) { - return false; - } - - return dst != null ? dst.equals(otherDst) : otherDst == null; - } - - public boolean isCompletedBy(final Trigger otherTrigger, final TriggerPhase otherTriggerPhase, final TriggerEvent otherTriggerEvent, final XmldbURI otherSrc, @Nullable final XmldbURI otherDst) { - if (this.triggerPhase != TriggerPhase.BEFORE - || otherTriggerPhase != TriggerPhase.AFTER) { - return false; - } - - if (!trigger.equals(otherTrigger)) { - return false; - } - - if (triggerEvent != otherTriggerEvent) { - return false; - } - - if (!src.equals(otherSrc)) { - return false; - } - - return dst != null ? dst.equals(otherDst) : otherDst == null; - } - - public boolean completes(final Object o) { - if (this == o) { - return false; - } - - if (o == null || getClass() != o.getClass()) { - return false; - } - - final TriggerState that = (TriggerState) o; - - if (this.triggerPhase != TriggerPhase.AFTER - || that.triggerPhase != TriggerPhase.BEFORE) { - return false; - } - - if (!trigger.equals(that.trigger)) { - return false; - } - - if (triggerEvent != that.triggerEvent) { - return false; - } - - if (!src.equals(that.src)) { - return false; - } - - return dst != null ? dst.equals(that.dst) : that.dst == null; - } - } + + private static final ThreadLocal> THREAD_LOCAL_STATES = ThreadLocal.withInitial(ArrayDeque::new); + + public static void setAndTest(final Trigger trigger, final TriggerPhase triggerPhase, final TriggerEvent triggerEvent, final XmldbURI src, final @Nullable XmldbURI dst) throws CyclicTriggerException { + final Deque states = THREAD_LOCAL_STATES.get(); + + if (states.isEmpty()) { + if (triggerPhase != TriggerPhase.BEFORE) { + throw new IllegalStateException("The Before phase of a trigger must occur before the After phase"); + } + states.addFirst(new TriggerState(trigger, triggerPhase, triggerEvent, src, dst, false)); + return; + } + + TriggerState prevState = states.peekFirst(); + + // is the new state the same as the previous state (excluding the phase) + if (prevState.equalsIgnoringPhase(trigger, triggerEvent, src, dst)) { + + // is this the after phase (i.e. matching completion) of a previous non-cyclic before phase? + if (triggerPhase == TriggerPhase.AFTER) { + + int skipBefores = 0; + + for (final Iterator it = states.iterator(); it.hasNext(); ) { + prevState = it.next(); + + // travel up, first "Before" we encounter - we should check if (a) that we complete it, and/or (b) is non-cyclic (if not we are also cyclic) + if (prevState.triggerPhase == TriggerPhase.BEFORE) { + + if (skipBefores > 0) { + skipBefores--; + + } else { + if (prevState.isCompletedBy(trigger, triggerPhase, triggerEvent, src, dst)) { + if (prevState.possiblyCyclic()) { + // if the Before phase is a PossibleCyclicTriggerState then this completing After phase must also be a PossibleCyclicTriggerState + final TriggerState newState = new TriggerState(trigger, triggerPhase, triggerEvent, src, dst, true); + states.addFirst(newState); + + throw new CyclicTriggerException("Detected Matching possible cyclic trigger event for After phase (" + newState + ") of previous Before phase (" + prevState + ")"); + + } else { + // if the Before Phase is NOT a PossibleCyclicTriggerState, then neither is this completing After phase... + states.addFirst(new TriggerState(trigger, triggerPhase, triggerEvent, src, dst, false)); + return; + } + + } else { + throw new IllegalStateException("Cannot interleave Trigger states"); + } + } + } else if (prevState.triggerPhase == TriggerPhase.AFTER) { + skipBefores++; + } + } + + throw new IllegalStateException("Could not find a matching Before phase for After phase"); + + } else { + // it's a cyclic exception! + final TriggerState newState = new TriggerState(trigger, triggerPhase, triggerEvent, src, dst, true); + states.addFirst(newState); + + throw new CyclicTriggerException("Detected possible cyclic trigger events: " + newState); + } + } + + states.addFirst(new TriggerState(trigger, triggerPhase, triggerEvent, src, dst, false)); + } + + public static class CyclicTriggerException extends Exception { + public CyclicTriggerException(final String message) { + super(message); + } + } + + public static void clearIfFinished(final TriggerPhase phase) { + if (phase == TriggerPhase.AFTER) { + + int depth = 0; + final Deque states = THREAD_LOCAL_STATES.get(); + for (final Iterator it = states.descendingIterator(); it.hasNext(); ) { + final TriggerState state = it.next(); + switch (state.triggerPhase) { + case BEFORE: + depth++; + break; + case AFTER: + depth--; + break; + default: + throw new IllegalStateException("Unknown phase: " + state.triggerPhase + "for trigger state: " + state); + } + } + + if (depth == 0) { + clear(); + } + } + } + + public static void clear() { + THREAD_LOCAL_STATES.remove(); + } + + public static boolean isEmpty() { + return THREAD_LOCAL_STATES.get().isEmpty(); + } + + private record TriggerState(Trigger trigger, TriggerPhase triggerPhase, TriggerEvent triggerEvent, XmldbURI src, + @Nullable XmldbURI dst, boolean possiblyCyclic) { + + @Override + public String toString() { + final StringBuilder builder = new StringBuilder(); + builder.append(triggerPhase); + builder.append(' '); + builder.append(triggerEvent); + builder.append('('); + if (triggerPhase == TriggerPhase.AFTER && dst != null) { + builder.append(dst); + builder.append(", "); + } + builder.append(src); + if (triggerPhase == TriggerPhase.BEFORE && dst != null) { + builder.append(", "); + builder.append(dst); + } + builder.append(')'); + builder.append(": "); + builder.append(trigger.getClass().getSimpleName()); + if (trigger instanceof XQueryTrigger queryTrigger) { + final String urlQuery = queryTrigger.getUrlQuery(); + if (urlQuery != null && !urlQuery.isEmpty()) { + builder.append('('); + builder.append(urlQuery); + builder.append(')'); + } + } + return builder.toString(); + } + + @Override + public boolean equals(final Object o) { + return equals(o, false); + } + + public boolean equalsIgnoringPhase(final Object o) { + return equals(o, true); + } + + private boolean equals(final Object o, final boolean ignorePhase) { + if (o instanceof TriggerState that) { + if (possiblyCyclic != that.possiblyCyclic) { + return false; + } + + if (!trigger.equals(that.trigger)) { + return false; + } + + if (!ignorePhase && + triggerPhase != that.triggerPhase) { + return false; + } + + if (triggerEvent != that.triggerEvent) { + return false; + } + + if (!src.equals(that.src)) { + return false; + } + + return Objects.equals(dst, that.dst); + } + return false; + } + + private boolean equalsIgnoringPhase(final Trigger otherTrigger, final TriggerEvent otherTriggerEvent, final XmldbURI otherSrc, @Nullable final XmldbURI otherDst) { + if (!trigger.equals(otherTrigger)) { + return false; + } + + if (triggerEvent != otherTriggerEvent) { + return false; + } + + if (!src.equals(otherSrc)) { + return false; + } + + return dst != null ? dst.equals(otherDst) : otherDst == null; + } + + public boolean isCompletedBy(final Trigger otherTrigger, final TriggerPhase otherTriggerPhase, final TriggerEvent otherTriggerEvent, final XmldbURI otherSrc, @Nullable final XmldbURI otherDst) { + if (this.triggerPhase != TriggerPhase.BEFORE + || otherTriggerPhase != TriggerPhase.AFTER) { + return false; + } + + if (!trigger.equals(otherTrigger)) { + return false; + } + + if (triggerEvent != otherTriggerEvent) { + return false; + } + + if (!src.equals(otherSrc)) { + return false; + } + + return dst != null ? dst.equals(otherDst) : otherDst == null; + } + + public boolean completes(final Object o) { + if (this == o) { + return false; + } + + if (o == null || getClass() != o.getClass()) { + return false; + } + + final TriggerState that = (TriggerState) o; + + if (this.triggerPhase != TriggerPhase.AFTER + || that.triggerPhase != TriggerPhase.BEFORE) { + return false; + } + + if (!trigger.equals(that.trigger)) { + return false; + } + + if (triggerEvent != that.triggerEvent) { + return false; + } + + if (!src.equals(that.src)) { + return false; + } + + return dst != null ? dst.equals(that.dst) : that.dst == null; + } + } } From 3d5bb50463fd82e7dac37ff74dcaa8468e673192 Mon Sep 17 00:00:00 2001 From: Patrick Reinhart Date: Tue, 30 Apr 2024 21:00:02 +0200 Subject: [PATCH 2/8] Removed unused methods, no longer use thread local - Use default 'equals' implementation of record - Store all thread related data within a central concurrent hash map in order to better track potential leaking data --- .../triggers/TriggerStatePerThread.java | 91 +++---------------- 1 file changed, 15 insertions(+), 76 deletions(-) diff --git a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java index eb69ab174dc..0faad32731d 100644 --- a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java +++ b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java @@ -28,6 +28,8 @@ import java.util.Deque; import java.util.Iterator; import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; /** * Avoid infinite recursions in Triggers by preventing the same trigger @@ -37,10 +39,10 @@ */ public class TriggerStatePerThread { - private static final ThreadLocal> THREAD_LOCAL_STATES = ThreadLocal.withInitial(ArrayDeque::new); + private static final ConcurrentMap> THREAD_LOCAL_STATES = new ConcurrentHashMap<>(); public static void setAndTest(final Trigger trigger, final TriggerPhase triggerPhase, final TriggerEvent triggerEvent, final XmldbURI src, final @Nullable XmldbURI dst) throws CyclicTriggerException { - final Deque states = THREAD_LOCAL_STATES.get(); + final Deque states = getStates(); if (states.isEmpty()) { if (triggerPhase != TriggerPhase.BEFORE) { @@ -117,7 +119,7 @@ public static void clearIfFinished(final TriggerPhase phase) { if (phase == TriggerPhase.AFTER) { int depth = 0; - final Deque states = THREAD_LOCAL_STATES.get(); + final Deque states = getStates(); for (final Iterator it = states.descendingIterator(); it.hasNext(); ) { final TriggerState state = it.next(); switch (state.triggerPhase) { @@ -139,14 +141,18 @@ public static void clearIfFinished(final TriggerPhase phase) { } public static void clear() { - THREAD_LOCAL_STATES.remove(); + THREAD_LOCAL_STATES.remove(Thread.currentThread()); } public static boolean isEmpty() { - return THREAD_LOCAL_STATES.get().isEmpty(); + return getStates().isEmpty(); } - private record TriggerState(Trigger trigger, TriggerPhase triggerPhase, TriggerEvent triggerEvent, XmldbURI src, + private static Deque getStates() { + return THREAD_LOCAL_STATES.computeIfAbsent(Thread.currentThread(), thread -> new ArrayDeque<>()); + } + + record TriggerState(Trigger trigger, TriggerPhase triggerPhase, TriggerEvent triggerEvent, XmldbURI src, @Nullable XmldbURI dst, boolean possiblyCyclic) { @Override @@ -179,44 +185,8 @@ public String toString() { return builder.toString(); } - @Override - public boolean equals(final Object o) { - return equals(o, false); - } - - public boolean equalsIgnoringPhase(final Object o) { - return equals(o, true); - } - - private boolean equals(final Object o, final boolean ignorePhase) { - if (o instanceof TriggerState that) { - if (possiblyCyclic != that.possiblyCyclic) { - return false; - } - - if (!trigger.equals(that.trigger)) { - return false; - } - - if (!ignorePhase && - triggerPhase != that.triggerPhase) { - return false; - } - - if (triggerEvent != that.triggerEvent) { - return false; - } - - if (!src.equals(that.src)) { - return false; - } - return Objects.equals(dst, that.dst); - } - return false; - } - - private boolean equalsIgnoringPhase(final Trigger otherTrigger, final TriggerEvent otherTriggerEvent, final XmldbURI otherSrc, @Nullable final XmldbURI otherDst) { + boolean equalsIgnoringPhase(final Trigger otherTrigger, final TriggerEvent otherTriggerEvent, final XmldbURI otherSrc, @Nullable final XmldbURI otherDst) { if (!trigger.equals(otherTrigger)) { return false; } @@ -229,7 +199,7 @@ private boolean equalsIgnoringPhase(final Trigger otherTrigger, final TriggerEve return false; } - return dst != null ? dst.equals(otherDst) : otherDst == null; + return Objects.equals(dst, otherDst); } public boolean isCompletedBy(final Trigger otherTrigger, final TriggerPhase otherTriggerPhase, final TriggerEvent otherTriggerEvent, final XmldbURI otherSrc, @Nullable final XmldbURI otherDst) { @@ -250,38 +220,7 @@ public boolean isCompletedBy(final Trigger otherTrigger, final TriggerPhase othe return false; } - return dst != null ? dst.equals(otherDst) : otherDst == null; - } - - public boolean completes(final Object o) { - if (this == o) { - return false; - } - - if (o == null || getClass() != o.getClass()) { - return false; - } - - final TriggerState that = (TriggerState) o; - - if (this.triggerPhase != TriggerPhase.AFTER - || that.triggerPhase != TriggerPhase.BEFORE) { - return false; - } - - if (!trigger.equals(that.trigger)) { - return false; - } - - if (triggerEvent != that.triggerEvent) { - return false; - } - - if (!src.equals(that.src)) { - return false; - } - - return dst != null ? dst.equals(that.dst) : that.dst == null; + return Objects.equals(dst, otherDst); } } } From d7f8accfadf70d53d81c24dcca551ed526b87029 Mon Sep 17 00:00:00 2001 From: Patrick Reinhart Date: Thu, 9 May 2024 23:12:14 +0200 Subject: [PATCH 3/8] Use transaction instead of thread --- .../triggers/TriggerStatePerThread.java | 51 ++++++++++++++----- .../collections/triggers/XQueryTrigger.java | 19 +++---- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java index 0faad32731d..b5c99909861 100644 --- a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java +++ b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java @@ -21,6 +21,8 @@ */ package org.exist.collections.triggers; +import org.exist.storage.txn.Txn; +import org.exist.storage.txn.TxnListener; import org.exist.xmldb.XmldbURI; import javax.annotation.Nullable; @@ -30,6 +32,8 @@ import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.function.BiConsumer; +import java.util.function.Consumer; /** * Avoid infinite recursions in Triggers by preventing the same trigger @@ -39,10 +43,10 @@ */ public class TriggerStatePerThread { - private static final ConcurrentMap> THREAD_LOCAL_STATES = new ConcurrentHashMap<>(); + private static final ConcurrentMap> TRIGGER_STATES = new ConcurrentHashMap<>(); - public static void setAndTest(final Trigger trigger, final TriggerPhase triggerPhase, final TriggerEvent triggerEvent, final XmldbURI src, final @Nullable XmldbURI dst) throws CyclicTriggerException { - final Deque states = getStates(); + public static void setAndTest(final Txn txn, final Trigger trigger, final TriggerPhase triggerPhase, final TriggerEvent triggerEvent, final XmldbURI src, final @Nullable XmldbURI dst) throws CyclicTriggerException { + final Deque states = getStates(txn); if (states.isEmpty()) { if (triggerPhase != TriggerPhase.BEFORE) { @@ -115,11 +119,11 @@ public CyclicTriggerException(final String message) { } } - public static void clearIfFinished(final TriggerPhase phase) { + public static void clearIfFinished(final Txn txn, final TriggerPhase phase) { if (phase == TriggerPhase.AFTER) { int depth = 0; - final Deque states = getStates(); + final Deque states = getStates(txn); for (final Iterator it = states.descendingIterator(); it.hasNext(); ) { final TriggerState state = it.next(); switch (state.triggerPhase) { @@ -135,24 +139,45 @@ public static void clearIfFinished(final TriggerPhase phase) { } if (depth == 0) { - clear(); + clear(txn); } } } - public static void clear() { - THREAD_LOCAL_STATES.remove(Thread.currentThread()); + public static void clear(final Txn txn) { + TRIGGER_STATES.remove(txn); } - public static boolean isEmpty() { - return getStates().isEmpty(); + public static boolean isEmpty(final Txn txn) { + return getStates(txn).isEmpty(); } - private static Deque getStates() { - return THREAD_LOCAL_STATES.computeIfAbsent(Thread.currentThread(), thread -> new ArrayDeque<>()); + public static void forEach(BiConsumer> action) { + TRIGGER_STATES.forEach(action); } - record TriggerState(Trigger trigger, TriggerPhase triggerPhase, TriggerEvent triggerEvent, XmldbURI src, + private static Deque getStates(final Txn txn) { + return TRIGGER_STATES.computeIfAbsent(txn, TriggerStatePerThread::initStates); + } + + private static Deque initStates(final Txn txn) { + txn.registerListener(new TransactionCleanUp(txn, TriggerStatePerThread::clear)); + return new ArrayDeque<>(); + } + + public record TransactionCleanUp(Txn txn, Consumer consumer) implements TxnListener { + @Override + public void commit() { + consumer.accept(txn); + } + + @Override + public void abort() { + consumer.accept(txn); + } + } + + public record TriggerState(Trigger trigger, TriggerPhase triggerPhase, TriggerEvent triggerEvent, XmldbURI src, @Nullable XmldbURI dst, boolean possiblyCyclic) { @Override diff --git a/exist-core/src/main/java/org/exist/collections/triggers/XQueryTrigger.java b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTrigger.java index ddde971e207..887d69e6eb1 100644 --- a/exist-core/src/main/java/org/exist/collections/triggers/XQueryTrigger.java +++ b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTrigger.java @@ -227,7 +227,7 @@ private void prepare(final TriggerEvent event, final DBBroker broker, final Txn // avoid infinite recursion try { - TriggerStatePerThread.setAndTest(this, TriggerPhase.BEFORE, event, src, dst); + TriggerStatePerThread.setAndTest(transaction,this, TriggerPhase.BEFORE, event, src, dst); } catch (final TriggerStatePerThread.CyclicTriggerException e) { LOG.warn(e.getMessage()); return; @@ -241,7 +241,7 @@ private void prepare(final TriggerEvent event, final DBBroker broker, final Txn declareExternalVariables(context, TriggerPhase.BEFORE, event, src, dst, isCollection); } catch (final XPathException | IOException | PermissionDeniedException e) { - TriggerStatePerThread.clear(); + TriggerStatePerThread.clear(transaction); throw new TriggerException(PREPARE_EXCEPTION_MESSAGE, e); } @@ -255,7 +255,7 @@ private void prepare(final TriggerEvent event, final DBBroker broker, final Txn LOG.debug("Trigger fired for prepare"); } } catch (final XPathException | PermissionDeniedException e) { - TriggerStatePerThread.clear(); + TriggerStatePerThread.clear(transaction); throw new TriggerException(PREPARE_EXCEPTION_MESSAGE, e); } finally { context.runCleanupTasks(); @@ -271,7 +271,7 @@ private void finish(final TriggerEvent event, final DBBroker broker, final Txn t // avoid infinite recursion try { - TriggerStatePerThread.setAndTest(this, TriggerPhase.AFTER, event, src, dst); + TriggerStatePerThread.setAndTest(transaction,this, TriggerPhase.AFTER, event, src, dst); } catch (final TriggerStatePerThread.CyclicTriggerException e) { LOG.warn(e.getMessage()); return; @@ -305,7 +305,7 @@ private void finish(final TriggerEvent event, final DBBroker broker, final Txn t context.runCleanupTasks(); } - TriggerStatePerThread.clearIfFinished(TriggerPhase.AFTER); + TriggerStatePerThread.clearIfFinished(transaction, TriggerPhase.AFTER); if (LOG.isDebugEnabled()) { LOG.debug("Trigger fired for finish"); @@ -393,10 +393,11 @@ private CompiledXQuery getScript(final DBBroker broker, final Txn transaction) t } private void execute(final TriggerPhase phase, final TriggerEvent event, final DBBroker broker, final Txn transaction, final QName functionName, final XmldbURI src, final XmldbURI dst) throws TriggerException { + System.err.format("phase: %s, event: %s, tx: %s, thread: %s", phase, event, transaction, Thread.currentThread()).println(); // avoid infinite recursion try { - TriggerStatePerThread.setAndTest(this, phase, event, src, dst); + TriggerStatePerThread.setAndTest(transaction, this, phase, event, src, dst); } catch (final TriggerStatePerThread.CyclicTriggerException e) { LOG.warn("Skipping Trigger: {}", e.getMessage()); return; @@ -414,7 +415,7 @@ private void execute(final TriggerPhase phase, final TriggerEvent event, final D return; } } catch (final TriggerException e) { - TriggerStatePerThread.clear(); + TriggerStatePerThread.clear(transaction); throw e; } @@ -454,14 +455,14 @@ private void execute(final TriggerPhase phase, final TriggerEvent event, final D } } - TriggerStatePerThread.clear(); + TriggerStatePerThread.clear(transaction); throw new TriggerException(PREPARE_EXCEPTION_MESSAGE, e); } finally { compiledQuery.reset(); context.runCleanupTasks(); } - TriggerStatePerThread.clearIfFinished(phase); + TriggerStatePerThread.clearIfFinished(transaction, phase); if (LOG.isDebugEnabled()) { if (phase == TriggerPhase.AFTER) { From 8cb77836b3e45660b2dbc9f48daf2ecf569f3459 Mon Sep 17 00:00:00 2001 From: Patrick Reinhart Date: Tue, 14 May 2024 07:06:16 +0200 Subject: [PATCH 4/8] Adds mbean server to track trigger states --- .../triggers/TriggerStatePerThread.java | 67 +++++++++++++++---- .../collections/triggers/XQueryTrigger.java | 6 +- .../triggers/XQueryTriggerMBean.java | 11 +++ .../triggers/XQueryTriggerMBeanImpl.java | 54 +++++++++++++++ 4 files changed, 125 insertions(+), 13 deletions(-) create mode 100644 exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBean.java create mode 100644 exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBeanImpl.java diff --git a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java index b5c99909861..8fe507082e4 100644 --- a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java +++ b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java @@ -26,12 +26,14 @@ import org.exist.xmldb.XmldbURI; import javax.annotation.Nullable; +import java.lang.ref.WeakReference; import java.util.ArrayDeque; +import java.util.Collections; import java.util.Deque; import java.util.Iterator; +import java.util.Map; import java.util.Objects; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; +import java.util.WeakHashMap; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -42,11 +44,10 @@ * @author Adam Retter */ public class TriggerStatePerThread { - - private static final ConcurrentMap> TRIGGER_STATES = new ConcurrentHashMap<>(); + private static final Map TRIGGER_STATES = Collections.synchronizedMap(new WeakHashMap<>()); public static void setAndTest(final Txn txn, final Trigger trigger, final TriggerPhase triggerPhase, final TriggerEvent triggerEvent, final XmldbURI src, final @Nullable XmldbURI dst) throws CyclicTriggerException { - final Deque states = getStates(txn); + final TriggerStates states = getStates(txn); if (states.isEmpty()) { if (triggerPhase != TriggerPhase.BEFORE) { @@ -123,7 +124,7 @@ public static void clearIfFinished(final Txn txn, final TriggerPhase phase) { if (phase == TriggerPhase.AFTER) { int depth = 0; - final Deque states = getStates(txn); + final TriggerStates states = getStates(txn); for (final Iterator it = states.descendingIterator(); it.hasNext(); ) { final TriggerState state = it.next(); switch (state.triggerPhase) { @@ -144,25 +145,37 @@ public static void clearIfFinished(final Txn txn, final TriggerPhase phase) { } } + public static int keys() { + return TRIGGER_STATES.size(); + } + + public static void clearAll() { + TRIGGER_STATES.clear(); + } + public static void clear(final Txn txn) { - TRIGGER_STATES.remove(txn); + TRIGGER_STATES.remove(Thread.currentThread()); } public static boolean isEmpty(final Txn txn) { return getStates(txn).isEmpty(); } - public static void forEach(BiConsumer> action) { + public static void dumpTriggerStates() { + TRIGGER_STATES.forEach((k, s) -> System.err.format("key: %s, size: %s", k, s.size()).println()); + } + + public static void forEach(BiConsumer action) { TRIGGER_STATES.forEach(action); } - private static Deque getStates(final Txn txn) { - return TRIGGER_STATES.computeIfAbsent(txn, TriggerStatePerThread::initStates); + private static TriggerStates getStates(final Txn txn) { + return TRIGGER_STATES.computeIfAbsent(Thread.currentThread(), key -> new TriggerStates()); } - private static Deque initStates(final Txn txn) { + private static TriggerStates initStates(final Txn txn) { txn.registerListener(new TransactionCleanUp(txn, TriggerStatePerThread::clear)); - return new ArrayDeque<>(); + return new TriggerStates(); } public record TransactionCleanUp(Txn txn, Consumer consumer) implements TxnListener { @@ -177,6 +190,36 @@ public void abort() { } } + public static final class TriggerStates extends WeakReference> { + public TriggerStates() { + super(new ArrayDeque<>()); + } + + public Iterator descendingIterator() { + return get().descendingIterator(); + } + + public boolean isEmpty() { + return get().isEmpty(); + } + + public int size() { + return get().size(); + } + + public Iterator iterator() { + return get().iterator(); + } + + public TriggerState peekFirst() { + return get().peekFirst(); + } + + public void addFirst(TriggerState newState) { + get().addFirst(newState); + } + } + public record TriggerState(Trigger trigger, TriggerPhase triggerPhase, TriggerEvent triggerEvent, XmldbURI src, @Nullable XmldbURI dst, boolean possiblyCyclic) { diff --git a/exist-core/src/main/java/org/exist/collections/triggers/XQueryTrigger.java b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTrigger.java index 887d69e6eb1..c53d02323ba 100644 --- a/exist-core/src/main/java/org/exist/collections/triggers/XQueryTrigger.java +++ b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTrigger.java @@ -119,7 +119,11 @@ public class XQueryTrigger extends SAXTrigger implements DocumentTrigger, Collec private String bindingPrefix = null; private XQuery service; - public final static String PREPARE_EXCEPTION_MESSAGE = "Error during trigger prepare"; + public static final String PREPARE_EXCEPTION_MESSAGE = "Error during trigger prepare"; + + public XQueryTrigger() { + XQueryTriggerMBeanImpl.init(); + } @Override public void configure(final DBBroker broker, final Txn transaction, final Collection parent, final Map> parameters) throws TriggerException { diff --git a/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBean.java b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBean.java new file mode 100644 index 00000000000..f38b8de6092 --- /dev/null +++ b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBean.java @@ -0,0 +1,11 @@ +package org.exist.collections.triggers; + +public interface XQueryTriggerMBean { + int getKeys(); + + void clear(); + + String dumpTriggerStates(); + + String listKeys(); +} diff --git a/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBeanImpl.java b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBeanImpl.java new file mode 100644 index 00000000000..7b439cf059f --- /dev/null +++ b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBeanImpl.java @@ -0,0 +1,54 @@ +package org.exist.collections.triggers; + +import javax.management.InstanceAlreadyExistsException; +import javax.management.MBeanRegistrationException; +import javax.management.MBeanServer; +import javax.management.MalformedObjectNameException; +import javax.management.NotCompliantMBeanException; +import javax.management.ObjectName; +import javax.management.StandardMBean; +import java.lang.management.ManagementFactory; +import java.util.StringJoiner; + +final class XQueryTriggerMBeanImpl extends StandardMBean implements XQueryTriggerMBean { + + private XQueryTriggerMBeanImpl() throws NotCompliantMBeanException { + super(XQueryTriggerMBean.class); + } + + static void init() { + try { + final ObjectName name = ObjectName.getInstance("org.exist.management.exist", "type", "TriggerStates"); + final MBeanServer platformMBeanServer = ManagementFactory.getPlatformMBeanServer(); + if (!platformMBeanServer.isRegistered(name)) { + platformMBeanServer.registerMBean(new XQueryTriggerMBeanImpl(), name); + } + } catch (final MalformedObjectNameException | InstanceAlreadyExistsException | MBeanRegistrationException | NotCompliantMBeanException ex) { + ex.printStackTrace(); + } + } + + @Override + public int getKeys() { + return TriggerStatePerThread.keys(); + } + + @Override + public void clear() { + TriggerStatePerThread.clearAll(); + } + + @Override + public String dumpTriggerStates() { + StringJoiner joiner = new StringJoiner("\n"); + TriggerStatePerThread.forEach((k, v) -> joiner.add("%s: %s".formatted(k, v.size()))); + return joiner.toString(); + } + + @Override + public String listKeys() { + StringJoiner joiner = new StringJoiner("\n"); + TriggerStatePerThread.forEach((k, v) -> joiner.add("%s".formatted(k))); + return joiner.toString(); + } +} From 728a89406f6f6e5ef2f7782e217dddf644c1b6e9 Mon Sep 17 00:00:00 2001 From: Patrick Reinhart Date: Tue, 14 May 2024 21:23:29 +0200 Subject: [PATCH 5/8] Fixes gone reference handling --- .../triggers/TriggerStatePerThread.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java index 8fe507082e4..a3ec0a92d8e 100644 --- a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java +++ b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java @@ -33,6 +33,7 @@ import java.util.Iterator; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.WeakHashMap; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -178,7 +179,7 @@ private static TriggerStates initStates(final Txn txn) { return new TriggerStates(); } - public record TransactionCleanUp(Txn txn, Consumer consumer) implements TxnListener { + public record TransactionCleanUp(Txn txn, Consumer consumer) implements TxnListener { @Override public void commit() { consumer.accept(txn); @@ -195,33 +196,37 @@ public TriggerStates() { super(new ArrayDeque<>()); } + Optional> states() { + return Optional.ofNullable(get()); + } + public Iterator descendingIterator() { - return get().descendingIterator(); + return states().map(Deque::descendingIterator).orElseGet(Collections::emptyIterator); } public boolean isEmpty() { - return get().isEmpty(); + return states().map(Deque::isEmpty).orElse(true); } public int size() { - return get().size(); + return states().map(Deque::size).orElse(0); } public Iterator iterator() { - return get().iterator(); + return states().map(Deque::iterator).orElseGet(Collections::emptyIterator); } public TriggerState peekFirst() { - return get().peekFirst(); + return states().map(Deque::peekFirst).orElse(null); } public void addFirst(TriggerState newState) { - get().addFirst(newState); + states().ifPresent(states -> states.addFirst(newState)); } } public record TriggerState(Trigger trigger, TriggerPhase triggerPhase, TriggerEvent triggerEvent, XmldbURI src, - @Nullable XmldbURI dst, boolean possiblyCyclic) { + @Nullable XmldbURI dst, boolean possiblyCyclic) { @Override public String toString() { From 1473c215e2a3b8ef773a2f4090882a2738f59c79 Mon Sep 17 00:00:00 2001 From: Patrick Reinhart Date: Tue, 14 May 2024 21:25:07 +0200 Subject: [PATCH 6/8] Fix missing headers --- .../triggers/XQueryTriggerMBean.java | 21 +++++++++++++++++++ .../triggers/XQueryTriggerMBeanImpl.java | 21 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBean.java b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBean.java index f38b8de6092..2439eb40da9 100644 --- a/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBean.java +++ b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBean.java @@ -1,3 +1,24 @@ +/* + * eXist-db Open Source Native XML Database + * Copyright (C) 2001 The eXist-db Authors + * + * info@exist-db.org + * http://www.exist-db.org + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ package org.exist.collections.triggers; public interface XQueryTriggerMBean { diff --git a/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBeanImpl.java b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBeanImpl.java index 7b439cf059f..0f8310f1faa 100644 --- a/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBeanImpl.java +++ b/exist-core/src/main/java/org/exist/collections/triggers/XQueryTriggerMBeanImpl.java @@ -1,3 +1,24 @@ +/* + * eXist-db Open Source Native XML Database + * Copyright (C) 2001 The eXist-db Authors + * + * info@exist-db.org + * http://www.exist-db.org + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ package org.exist.collections.triggers; import javax.management.InstanceAlreadyExistsException; From 490e687fc58554f5749fe9312fa04a17993d5f5b Mon Sep 17 00:00:00 2001 From: Patrick Reinhart Date: Mon, 27 May 2024 20:11:41 +0200 Subject: [PATCH 7/8] Not remove referenced dqueue values --- .../exist/collections/triggers/TriggerStatePerThread.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java index a3ec0a92d8e..4c27823becb 100644 --- a/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java +++ b/exist-core/src/main/java/org/exist/collections/triggers/TriggerStatePerThread.java @@ -30,6 +30,7 @@ import java.util.ArrayDeque; import java.util.Collections; import java.util.Deque; +import java.util.HashSet; import java.util.Iterator; import java.util.Map; import java.util.Objects; @@ -191,8 +192,11 @@ public void abort() { } } - public static final class TriggerStates extends WeakReference> { - public TriggerStates() { + public static final class TriggerStates extends ArrayDeque { + } + + public static final class TriggerStatesX extends WeakReference> { + public TriggerStatesX() { super(new ArrayDeque<>()); } From f301b5fb8b536f2413cb421ca8358d0bee0490f5 Mon Sep 17 00:00:00 2001 From: Patrick Reinhart Date: Mon, 27 May 2024 20:16:57 +0200 Subject: [PATCH 8/8] Disable triggers during restore --- .../src/main/java/org/exist/backup/Restore.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/exist-core/src/main/java/org/exist/backup/Restore.java b/exist-core/src/main/java/org/exist/backup/Restore.java index 7b38622b112..9c4de22c58a 100644 --- a/exist-core/src/main/java/org/exist/backup/Restore.java +++ b/exist-core/src/main/java/org/exist/backup/Restore.java @@ -76,9 +76,11 @@ public void restore(final DBBroker broker, @Nullable final Txn transaction, fina } // continue restore - final XMLReaderPool parserPool = broker.getBrokerPool().getParserPool(); + final XMLReaderPool parserPool = broker.getBrokerPool().getXmlReaderPool(); + final boolean triggersEnabled = broker.isTriggersEnabled(); XMLReader reader = null; try { + broker.setTriggersEnabled(false); reader = parserPool.borrowXMLReader(); listener.started(totalNrOfFiles); @@ -99,10 +101,14 @@ public void restore(final DBBroker broker, @Nullable final Txn transaction, fina } } finally { - listener.finished(); + try { + listener.finished(); + } finally { + broker.setTriggersEnabled(triggersEnabled); - if (reader != null) { - parserPool.returnXMLReader(reader); + if (reader != null) { + parserPool.returnXMLReader(reader); + } } } }