Merged
Conversation
balloob
approved these changes
Oct 1, 2018
Member
balloob
left a comment
There was a problem hiding this comment.
One potential race condition could be that the config entry set up is not yet listening to dispatched signals.
This fix is a good start though.
3 tasks
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
After looking into #16968 a bit more (so that the device registry will be possible for MQTT entities 😉), and finding out what exactly was causing the problem, I think I now have a working fix that does not modify any core code.
The problem was that the MQTT discovery callback would get called multiple times while the platform was still being set up. Inside
async_forward_entry_setupand then inasync_setup_componenta new task is created for the loading of each component. Until the config entry is fully forwarded, another MQTT discovery message might come in and callasync_forward_entry_setupanother time, triggering the bug in #16968Suppose a discovery message for a
switch.mqttcomes in. The platform needs to be loaded and the config entry is forwarded viaasync_forward_entry_setup.async_forward_entry_setuphowever adds new tasks to the event queue and the key is only added toCONFIG_ENTRY_IS_SETUPafter that newly created task is completed.During that time, another discovery message for another switch might come in. Nothing is in
CONFIG_ENTRY_IS_SETUPyet so the platform is loaded again, triggering #16968This PR shuffles the affected line around and I have since not been able to reproduce the race condition. Because this code fragment is now executed in one go and lives inside the event queue, I think the race condition should be fixed.
Related issue (if applicable): fixes #16968
Checklist:
tox. Your PR cannot be merged unless tests passIf the code does not interact with devices: