Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

concurrent map read and map write #5733

Closed
mrwilby opened this issue Mar 19, 2016 · 6 comments
Closed

concurrent map read and map write #5733

mrwilby opened this issue Mar 19, 2016 · 6 comments

Comments

@mrwilby
Copy link

mrwilby commented Mar 19, 2016

We have a 'reasonably complex' project structure - a lot of modules, null_resources and other goodies to help modularize our environment.

Since upgrading to 0.6.11 and 0.6.12 (could've been earlier too - but we really started noticing the problem lately) we now see the terraform process completely die with exit code 2 - there's basically no output at all (unless you turn on debug logging) - the process just bombs without showing anything.

I managed to get a trace log of the issue today and captured the call stacks which I'll add here:

https://gist.github.com/mrwilby/e78ed1274a46c54ae3f9

I have the full trace log but I am not willing to post that in public as it contains a lot of project information.

@jen20
Copy link
Contributor

jen20 commented Mar 21, 2016

Thanks for reporting this @mrwilby! This is definitely a core bug - potentially introduced when hil was factored out into it's own repository and separated out from Terraform's core, though I forget the exact timelines. The stack trace should be sufficient to debug this though.

phinze added a commit that referenced this issue Mar 21, 2016
The graph walker holds a lock that's passed down to BuiltinEvalContext
and guards access to interpolation variables as they're written using
SetVariables.

The likely problem being expressed in #5733 is that the same map
reference is also passed down to the Interpolater.Variables field, which
is used for variable lookup.

Here, we plumb the same lock we're using to guard access for writes down
and acquire it before doing variable reads as well. It's not as fine
grained as perhaps it could be, but all the context tests pass and I
believe this should address #5733.
phinze added a commit that referenced this issue Mar 21, 2016
The ContextGraphWalker struct includes a lock that's passed down to
BuiltinEvalContext and guards access to interpolation variables as
they're written using SetVariables.

The likely problem being expressed in #5733 is that the same map
reference is also passed down to the Interpolater.Variables field, which
is used for variable lookup.

Here, we plumb the same lock we're using to guard access for writes down
and acquire it before doing variable reads as well. It's not as fine
grained as perhaps it could be, but all the context tests pass and I
believe this should address #5733.
@roderickrandolph
Copy link

experienced the same problem - 0.6.14 seems to fix it. thank you!

@phinze
Copy link
Contributor

phinze commented Mar 22, 2016

That's great news @roderickrandolph! #5772 did land in 0.6.14 and attempts to address this - @mrwilby can you give 0.6.14 a shot as well and let me know if it sorts out your issue?

dharrisio pushed a commit to dharrisio/terraform that referenced this issue Mar 29, 2016
The ContextGraphWalker struct includes a lock that's passed down to
BuiltinEvalContext and guards access to interpolation variables as
they're written using SetVariables.

The likely problem being expressed in hashicorp#5733 is that the same map
reference is also passed down to the Interpolater.Variables field, which
is used for variable lookup.

Here, we plumb the same lock we're using to guard access for writes down
and acquire it before doing variable reads as well. It's not as fine
grained as perhaps it could be, but all the context tests pass and I
believe this should address hashicorp#5733.
@mrwilby
Copy link
Author

mrwilby commented Apr 12, 2016

Sorry for the delay. This does indeed look good now with 0.6.14. Thanks!

@phinze
Copy link
Contributor

phinze commented Apr 12, 2016

Hooray! Closing 👍

@phinze phinze closed this as completed Apr 12, 2016
@ghost
Copy link

ghost commented Apr 26, 2020

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 ghost locked and limited conversation to collaborators Apr 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants