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

Stop downloading the app asynchronously #87

Conversation

bgandon
Copy link

@bgandon bgandon commented May 30, 2018

Hi,

Here in this PR, I'm submitting the fix by Jim Carrothers @jcarrothers-sap (as filed in meeting notes from Monday April the 16th, 2018) that fixes a bug when downloading application files asynchronously in resourceAppCreate().

I submit this because the bug is quite severe and the fix wasn't proposed here in upstream yet.

Here are Jim's own words ab out the issue:

The problem is that the schema.ResourceData is not thread safe, even
for concurrent reads, due to internal memoization during reads. The
result was that app creation would sometimes fail mysteriously with an
"unexpected EOF" error due to the provider process crashing due to
either concurrent map writes or read/write.

Best

The problem is that the schema.ResourceData is not thread safe, even
for concurrent reads, due to internal memoization during reads.  The
result was that app creation would sometimes fail mysteriously with an
"unexpected EOF" error due to the provider process crashing due to
either concurrent map writes or read/write.
@jcarrothers-sap
Copy link
Collaborator

I hadn't realized there was a serious demand for this fix, but I'd be happy to open the PR myself if people would prefer.

@janosbinder
Copy link
Collaborator

+1 @jcarrothers-sap 's fix was a great help for us.

Issue opened here: #88

@mevansam @gberche-orange @psycofdj @ArthurHlt could you please review this PR?

@janosbinder
Copy link
Collaborator

Hm, it seems that the acceptance environment has been not set up for travis as it is missing a lot of CF_... shell variables.

@jcarrothers-sap
Copy link
Collaborator

I would like to advocate for merging #103 in place of this, as it will merge more cleanly with #104

@bgandon
Copy link
Author

bgandon commented Jun 8, 2018

Of course no problem with this!

@bgandon bgandon closed this Jun 8, 2018
sleungcy pushed a commit to sleungcy/terraform-provider-cf that referenced this pull request Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants