home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.46k stars 30.69k forks source link

Automation `mode: restart` does not restart automation if multiple runs are triggered close in time #49171

Closed EPMatt closed 3 years ago

EPMatt commented 3 years ago

The problem

The automation run mode restart does not restart an automation run if a second one is triggered moments after the first one started.

This issue was encountered while debugging a blueprint (https://github.com/EPMatt/awesome-ha-blueprints/issues/20). OP noticed that the automation generated by the blueprint was not restarted, even if mode: restart was set in the blueprint config.

I was able to craft a faulty automation to reproduce the issue only by using a script, custom events and the persistent_notification integration.

The faulty automation, configured with mode: restart, waits 5 seconds, then creates a persistent notification in the HA frontend. The script simply triggers the automation 10 times in a row without waiting single runs to complete.

To reproduce

  1. Copy the provided YAML config in the scripts.yaml and automations.yaml of your instance.
  2. Reload automations and scripts.
  3. Run the test_issue script.

Expected Behaviour

Only one completed notification is created in the frontend. That means that only an automation run completes execution and the automation was succesfully restarted, every time a new run was spawned, as mode: restart is expected to work.

Actual Behaviour

Two or more completed notifications are created in the frontend. That means that at least one automation run was not terminated when the next one was spawned; instead it was able to complete execution, even if mode: restart is set in the automation config.

What is version of Home Assistant Core has the issue?

core-2021.4.3

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

automation

Link to integration documentation on our website

https://www.home-assistant.io/docs/automation/

Example YAML snippet

### automations.yaml - faulty automation: wait 5 seconds then create a notification

- id: faulty_automation
  mode: restart
  trigger:
  - platform: event
    event_type: MY_EVENT
  condition: []
  action:
  - delay:
      seconds: 5
  - service: persistent_notification.create
    data:
      message: completed

# scripts.yaml - script to trigger automation 10 times in a row

test_issue:
  mode: single
  sequence:
    - repeat:
        while: '{{ repeat.index < 10 }}'
        sequence:
        - event: MY_EVENT

Anything in the logs that might be useful for us?

No response

tdejneka commented 3 years ago

I'm not in position to fix this Issue but thought you might like to know I tried your suggested automation and script and can confirm it produced two notifications. I modified the service call to include a timestamp in the notification. The two notifications report the exact same time (to the second).

I agree this is unexpected; seems there should only be one notification. On the other hand, if this is working to spec then it would be useful to know why.

probot-home-assistant[bot] commented 3 years ago

automation documentation automation source (message by IssueLinks)

probot-home-assistant[bot] commented 3 years ago

Hey there @home-assistant/core, mind taking a look at this issue as its been labeled with an integration (automation) you are listed as a codeowner for? Thanks! (message by CodeOwnersMention)

EPMatt commented 3 years ago

Hi @tdejneka, thank you for testing this by yourself.

It's interesting that the two automation runs look to be fired exactly at the same time, but to the second. This info might help developers to identify the problem. 👍🏻

emontnemery commented 3 years ago

~~Could you change the script to trigger only 2 times, then share the resulting 2 automation traces? Edit: Or does running the script, which loops 10 times create two persistent notifications, not 10?~~

Never mind about this, I can reproduce it.

emontnemery commented 3 years ago

I think there's a race here: https://github.com/home-assistant/core/blob/dev/homeassistant/helpers/script.py#L1149

EPMatt commented 3 years ago

Hi @emontnemery,

~Could you change the script to trigger only 2 times, then share the resulting 2 automation traces? Edit: Or does running the script, which loops 10 times create two persistent notifications, not 10?~

Never mind about this, I can reproduce it.

by the way, I tried to tweak the number of automation triggers in the provided script, and it looks like only firing 4+ automations produces 2 notifications. I tested it also with 100 automation triggers and the result is still the same - two separate runs complete execution.