core: Make copies when creating destroy nodes #5026
Merged
Conversation
This file contains 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
phinze
force-pushed
the
phinze/destroy-node-copies
branch
from
7dcdf37
to
d927f66
Compare
|
Added a test covering the race condition. Ping @mitchellh for review on the golang-isms here. 👀 |
|
LGTM. You may want to test I also will take a look at why copystructure doesn't work here (it doesn't), because we should probably use that for simplicity. But Merge this for now! |
Fixes an interpolation race that was occurring when a tainted destroy node and a primary destroy node both tried to interpolate a computed count in their config. Since they were sharing a pointer to the _same_ config, depending on how the race played out one of them could catch the config uninterpolated and would then throw a syntax error. The `Copy()` tree implemented for this fix can probably be used elsewhere - basically we should copy the config whenever we drop nodes into the graph - but for now I'm just applying it to the place that fixes this bug. Fixes #4982 - Includes a test covering that race condition.
phinze
force-pushed
the
phinze/destroy-node-copies
branch
from
d927f66
to
3f72837
Compare
|
Okay |
phinze
added a commit
that referenced
this pull request
Feb 9, 2016
core: Make copies when creating destroy nodes
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes an interpolation race that was occurring when a tainted destroy
node and a primary destroy node both tried to interpolate a computed
count in their config. Since they were sharing a pointer to the same
config, depending on how the race played out one of them could catch the
config uninterpolated and would then throw a syntax error.
The
Copy()tree implemented for this fix can probably be usedelsewhere - basically we should copy the config whenever we drop nodes
into the graph - but for now I'm just applying it to the place that
fixes this bug.
Fixes #4982