Use a thread pool backed sequential executor for DSL rules and events

Signed-off-by: Jörg Sautter <[email protected]>

bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/automation/MarketplaceRuleTemplateProvider.java

@@ -117,7 +117,7 @@ public void addTemplateAsJSON(String uid, String json) throws ParsingException {
                 entry.getTags().add(uid);
                 RuleTemplate template = new RuleTemplate(entry.getUID(), entry.getLabel(), entry.getDescription(),
                         entry.getTags(), entry.getTriggers(), entry.getConditions(), entry.getActions(),
-                        entry.getConfigurationDescriptions(), entry.getVisibility());
+                        entry.getConfigurationDescriptions(), entry.getVisibility(), entry.isThreadBound());
                 add(template);
             }
         } catch (IOException e) {
@@ -142,7 +142,7 @@ public void addTemplateAsYAML(String uid, String yaml) throws ParsingException {
             RuleTemplate entry = RuleTemplateDTOMapper.map(dto);
             RuleTemplate template = new RuleTemplate(entry.getUID(), entry.getLabel(), entry.getDescription(),
                     entry.getTags(), entry.getTriggers(), entry.getConditions(), entry.getActions(),
-                    entry.getConfigurationDescriptions(), entry.getVisibility());
+                    entry.getConfigurationDescriptions(), entry.getVisibility(), entry.isThreadBound());
             add(template);
         } catch (IOException e) {
             logger.error("Unable to parse YAML: {}", e.getMessage());

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/Rule.java

@@ -154,4 +154,12 @@ public interface Rule extends Identifiable<String> {
         }
         return null;
     }
+
+    /**
+     * Is it necessary to run this rule in the same thread all the time, e.g. due to usage of
+     * {@link java.lang.ThreadLocal}.
+     *
+     * @return true when this rule must en executed in the same thread, false if not and a thread pool can be used.
+     */
+    boolean isThreadBound();
 }

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/dto/RuleTemplateDTO.java

@@ -33,4 +33,5 @@ public class RuleTemplateDTO {
     public List<TriggerDTO> triggers;
     public List<ConditionDTO> conditions;
     public List<ActionDTO> actions;
+    public boolean threadBound = true;
 }

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/dto/RuleTemplateDTOMapper.java

@@ -34,7 +34,8 @@ public static RuleTemplate map(final RuleTemplateDTO ruleTemplateDto) {
         return new RuleTemplate(ruleTemplateDto.uid, ruleTemplateDto.label, ruleTemplateDto.description,
                 ruleTemplateDto.tags, TriggerDTOMapper.mapDto(ruleTemplateDto.triggers),
                 ConditionDTOMapper.mapDto(ruleTemplateDto.conditions), ActionDTOMapper.mapDto(ruleTemplateDto.actions),
-                ConfigDescriptionDTOMapper.map(ruleTemplateDto.configDescriptions), ruleTemplateDto.visibility);
+                ConfigDescriptionDTOMapper.map(ruleTemplateDto.configDescriptions), ruleTemplateDto.visibility,
+                ruleTemplateDto.threadBound);
     }
 
     protected static void fillProperties(final RuleTemplate from, final RuleTemplateDTO to) {
@@ -47,5 +48,6 @@ protected static void fillProperties(final RuleTemplate from, final RuleTemplate
         to.triggers = TriggerDTOMapper.map(from.getTriggers());
         to.conditions = ConditionDTOMapper.map(from.getConditions());
         to.actions = ActionDTOMapper.map(from.getActions());
+        to.threadBound = from.isThreadBound();
     }
 }

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/RuleEngineImpl.java

@@ -587,10 +587,10 @@ protected void postRuleStatusInfoEvent(String ruleUID, RuleStatusInfo statusInfo
      * @param ruleUID rule object for which the callback is looking for.
      * @return a {@link TriggerHandlerCallback} corresponding to the passed {@link Rule} object.
      */
-    private synchronized TriggerHandlerCallbackImpl getTriggerHandlerCallback(String ruleUID) {
+    private synchronized TriggerHandlerCallbackImpl getTriggerHandlerCallback(String ruleUID, boolean threadBound) {
         TriggerHandlerCallbackImpl result = thCallbacks.get(ruleUID);
         if (result == null) {
-            result = new TriggerHandlerCallbackImpl(this, ruleUID);
+            result = new TriggerHandlerCallbackImpl(this, ruleUID, threadBound);
             thCallbacks.put(ruleUID, result);
         }
         return result;
@@ -627,7 +627,7 @@ private synchronized TriggerHandlerCallbackImpl getTriggerHandlerCallback(String
     private void register(WrappedRule rule) {
         final String ruleUID = rule.getUID();
 
-        TriggerHandlerCallback thCallback = getTriggerHandlerCallback(ruleUID);
+        TriggerHandlerCallback thCallback = getTriggerHandlerCallback(ruleUID, rule.isThreadBound());
         rule.getTriggers().forEach(trigger -> {
             TriggerHandler triggerHandler = trigger.getModuleHandler();
             if (triggerHandler != null) {

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/RuleImpl.java

@@ -53,6 +53,7 @@ public class RuleImpl implements Rule {
     protected Set<String> tags;
     protected Visibility visibility;
     protected @Nullable String description;
+    protected boolean threadBound;
 
     /**
      * Constructor for creating an empty {@link Rule} with a specified rule identifier.
@@ -62,7 +63,7 @@ public class RuleImpl implements Rule {
      * @param uid the rule's identifier, or {@code null} if a random identifier should be generated.
      */
     public RuleImpl(@Nullable String uid) {
-        this(uid, null, null, null, null, null, null, null, null, null, null);
+        this(uid, null, null, null, null, null, null, null, null, null, null, true);
     }
 
     /**
@@ -86,11 +87,14 @@ public RuleImpl(@Nullable String uid) {
      *            {@link RuleRegistry} to validate the {@link Rule}'s configuration, as well as to create and configure
      *            the {@link Rule}'s modules, or null if the {@link Rule} should not be created from a template.
      * @param visibility the {@link Rule}'s visibility
+     * @param threadBound the {@link Rule}'s threading behaviour
+     *
      */
     public RuleImpl(@Nullable String uid, final @Nullable String name, final @Nullable String description,
             final @Nullable Set<String> tags, @Nullable List<Trigger> triggers, @Nullable List<Condition> conditions,
             @Nullable List<Action> actions, @Nullable List<ConfigDescriptionParameter> configDescriptions,
-            @Nullable Configuration configuration, @Nullable String templateUID, @Nullable Visibility visibility) {
+            @Nullable Configuration configuration, @Nullable String templateUID, @Nullable Visibility visibility,
+            boolean threadBound) {
         this.uid = uid == null ? UUID.randomUUID().toString() : uid;
         this.name = name;
         this.description = description;
@@ -103,6 +107,7 @@ public RuleImpl(@Nullable String uid, final @Nullable String name, final @Nullab
                 : new Configuration(configuration.getProperties());
         this.templateUID = templateUID;
         this.visibility = visibility == null ? Visibility.VISIBLE : visibility;
+        this.threadBound = threadBound;
     }
 
     @Override
@@ -260,6 +265,10 @@ public List<Module> getModules() {
         return Collections.unmodifiableList(modules);
     }
 
+    public boolean isThreadBound() {
+        return threadBound;
+    }
+
     @Override
     public int hashCode() {
         final int prime = 31;

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/TriggerHandlerCallbackImpl.java

@@ -24,6 +24,7 @@
 import org.openhab.core.automation.Trigger;
 import org.openhab.core.automation.handler.TriggerHandlerCallback;
 import org.openhab.core.common.NamedThreadFactory;
+import org.openhab.core.common.ThreadPoolManager;
 
 /**
  * This class is implementation of {@link TriggerHandlerCallback} used by the {@link Trigger}s to notify rule engine
@@ -45,10 +46,15 @@ public class TriggerHandlerCallbackImpl implements TriggerHandlerCallback {
 
     private @Nullable Future<?> future;
 
-    protected TriggerHandlerCallbackImpl(RuleEngineImpl re, String ruleUID) {
+    protected TriggerHandlerCallbackImpl(RuleEngineImpl re, String ruleUID, boolean threadBound) {
         this.re = re;
         this.ruleUID = ruleUID;
-        executor = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("rule-" + ruleUID));
+        if (!threadBound && Boolean.getBoolean("org.openhab.core.common.SequentialScheduledExecutorService")) {
+            executor = ThreadPoolManager
+                    .newSequentialScheduledExecutorService(re.getClass().getSimpleName() + "-rules");
+        } else {
+            executor = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("rule-" + ruleUID));
+        }
     }
 
     @Override

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/provider/TemplateResourceBundleProvider.java

@@ -179,7 +179,7 @@ public Collection<RuleTemplate> getTemplates(@Nullable Locale locale) {
             List<Trigger> ltriggers = moduleI18nUtil.getLocalizedModules(defTemplate.getTriggers(), bundle, uid,
                     RuleTemplateI18nUtil.RULE_TEMPLATE, locale);
             return new RuleTemplate(uid, llabel, ldescription, defTemplate.getTags(), ltriggers, lconditions, lactions,
-                    lconfigDescriptions, defTemplate.getVisibility());
+                    lconfigDescriptions, defTemplate.getVisibility(), defTemplate.isThreadBound());
         }
         return null;
     }

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ruleengine/WrappedRule.java

@@ -65,6 +65,10 @@ public final String getUID() {
         return rule.getUID();
     }
 
+    public final boolean isThreadBound() {
+        return rule.isThreadBound();
+    }
+
     public final Rule unwrap() {
         return rule;
     }

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/template/RuleTemplateRegistry.java

@@ -74,7 +74,8 @@ private RuleTemplate createCopy(RuleTemplate template) {
         return new RuleTemplate(template.getUID(), template.getLabel(), template.getDescription(),
                 new HashSet<>(template.getTags()), new ArrayList<>(template.getTriggers()),
                 new ArrayList<>(template.getConditions()), new ArrayList<>(template.getActions()),
-                new LinkedList<>(template.getConfigurationDescriptions()), template.getVisibility());
+                new LinkedList<>(template.getConfigurationDescriptions()), template.getVisibility(),
+                template.isThreadBound());
     }
 
     @Override

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/template/RuleTemplate.java

@@ -90,6 +90,11 @@ public class RuleTemplate implements Template {
      */
     private final List<ConfigDescriptionParameter> configDescriptions;
 
+    /**
+     * Defines the threading behaviour expected by the template.
+     */
+    private final boolean threadBound;
+
     /**
      * Creates a {@link RuleTemplate} instance that will be used for creating {@link Rule}s from a set of modules,
      * belong to the template. When {@code null} is passed for the {@code uid} parameter, the {@link RuleTemplate}'s
@@ -112,7 +117,7 @@ public class RuleTemplate implements Template {
     public RuleTemplate(@Nullable String uid, @Nullable String label, @Nullable String description,
             @Nullable Set<String> tags, @Nullable List<Trigger> triggers, @Nullable List<Condition> conditions,
             @Nullable List<Action> actions, @Nullable List<ConfigDescriptionParameter> configDescriptions,
-            @Nullable Visibility visibility) {
+            @Nullable Visibility visibility, boolean threadBound) {
         this.uid = uid == null ? UUID.randomUUID().toString() : uid;
         this.label = label;
         this.description = description;
@@ -123,6 +128,7 @@ public RuleTemplate(@Nullable String uid, @Nullable String label, @Nullable Stri
                 : Collections.unmodifiableList(configDescriptions);
         this.visibility = visibility == null ? Visibility.VISIBLE : visibility;
         this.tags = tags == null ? Set.of() : Collections.unmodifiableSet(tags);
+        this.threadBound = threadBound;
     }
 
     /**
@@ -257,6 +263,16 @@ public List<Action> getActions() {
         return actions;
     }
 
+    /**
+     * Is it necessary to run this rule in the same thread all the time, e.g. due to usage of
+     * {@link java.lang.ThreadLocal}.
+     *
+     * @return true when this rule must en executed in the same thread, false if not and a thread pool can be used.
+     */
+    public boolean isThreadBound() {
+        return threadBound;
+    }
+
     /**
      * Returns the hash code of this object depends on the hash code of the UID that it owns.
      */

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/util/RuleBuilder.java

@@ -46,6 +46,7 @@ public class RuleBuilder {
     private List<ConfigDescriptionParameter> configDescriptions;
     private @Nullable String templateUID;
     private final String uid;
+    private boolean threadBound;
     private @Nullable String name;
     private Set<String> tags;
     private @Nullable Visibility visibility;
@@ -63,6 +64,7 @@ protected RuleBuilder(Rule rule) {
         this.tags = new HashSet<>(rule.getTags());
         this.visibility = rule.getVisibility();
         this.description = rule.getDescription();
+        this.threadBound = rule.isThreadBound();
     }
 
     public static RuleBuilder create(String ruleId) {
@@ -74,15 +76,21 @@ public static RuleBuilder create(Rule r) {
         return create(r.getUID()).withActions(r.getActions()).withConditions(r.getConditions())
                 .withTriggers(r.getTriggers()).withConfiguration(r.getConfiguration())
                 .withConfigurationDescriptions(r.getConfigurationDescriptions()).withDescription(r.getDescription())
-                .withName(r.getName()).withTags(r.getTags());
+                .withName(r.getName()).withTags(r.getTags()).threadBound(r.isThreadBound());
     }
 
     public static RuleBuilder create(RuleTemplate template, String uid, @Nullable String name,
             Configuration configuration, Visibility visibility) {
         return create(uid).withActions(template.getActions()).withConditions(template.getConditions())
                 .withTriggers(template.getTriggers()).withConfiguration(configuration)
                 .withConfigurationDescriptions(template.getConfigurationDescriptions())
-                .withDescription(template.getDescription()).withName(name).withTags(template.getTags());
+                .withDescription(template.getDescription()).withName(name).withTags(template.getTags())
+                .threadBound(template.isThreadBound());
+    }
+
+    public RuleBuilder threadBound(boolean state) {
+        this.threadBound = state;
+        return this;
     }
 
     public RuleBuilder withName(@Nullable String name) {
@@ -166,6 +174,6 @@ public RuleBuilder withConfigurationDescriptions(@Nullable List<ConfigDescriptio
 
     public Rule build() {
         return new RuleImpl(uid, name, description, tags, triggers, conditions, actions, configDescriptions,
-                configuration, templateUID, visibility);
+                configuration, templateUID, visibility, threadBound);
     }
 }

bundles/org.openhab.core.model.rule.runtime/src/org/openhab/core/model/rule/runtime/internal/DSLRuleProvider.java

@@ -253,7 +253,8 @@ private Rule toRule(String modelName, Script script) {
         List<Action> actions = List.of(ActionBuilder.create().withId("script").withTypeUID("script.ScriptAction")
                 .withConfiguration(cfg).build());
 
-        return RuleBuilder.create(modelName).withTags("Script").withName(modelName).withActions(actions).build();
+        return RuleBuilder.create(modelName).withTags("Script").withName(modelName).withActions(actions)
+                .threadBound(false).build();
     }
 
     private Rule toRule(String modelName, org.openhab.core.model.rule.rules.Rule rule, int index) {
@@ -280,7 +281,8 @@ private Rule toRule(String modelName, org.openhab.core.model.rule.rules.Rule rul
         List<Action> actions = List.of(ActionBuilder.create().withId("script").withTypeUID("script.ScriptAction")
                 .withConfiguration(cfg).build());
 
-        return RuleBuilder.create(uid).withName(name).withTriggers(triggers).withActions(actions).build();
+        return RuleBuilder.create(uid).withName(name).withTriggers(triggers).withActions(actions).threadBound(false)
+                .build();
     }
 
     private String removeIndentation(String script) {