Skip to content

feat: Use a connection pool for curl to avoid TLS and retry on more errors#489

Merged
mitsuhiko merged 2 commits intomasterfrom
feature/curl-pool
Feb 25, 2019
Merged

feat: Use a connection pool for curl to avoid TLS and retry on more errors#489
mitsuhiko merged 2 commits intomasterfrom
feature/curl-pool

Conversation

@mitsuhiko
Copy link
Copy Markdown
Contributor

@mitsuhiko mitsuhiko commented Feb 22, 2019

We're pretty confident that curl handles stashed into TLS causes all
kinds of mischief on thread shutdown which sometimes might even cause a
segfault. This instead stashes the handles into an r2d2 connection
pool.

This PR also adds more retries for common network issues we encounter.

We're pretty confident that curl handles stashed into TLS causes all
kinds of mischief on thread shutdown which sometimes might even cause a
segfault.  This instead stashes the handles into an r2d2 connection
pool.

This also removes the manual cleanup now which we added as a workaround
because TLS did not trigger dtors.
@mitsuhiko mitsuhiko requested a review from jan-auer February 22, 2019 20:42
@mitsuhiko
Copy link
Copy Markdown
Contributor Author

I'm thinking that instead of using a lazy static here it would be safer to create it in the main function. That would also cause it to actually clean up.

@mitsuhiko mitsuhiko changed the title feat: Use a connection pool for curl to avoid TLS feat: Use a connection pool for curl to avoid TLS and retry on more errors Feb 24, 2019
@jan-auer
Copy link
Copy Markdown
Member

Agreed, allocating somewhere around main and passing it down would be nicer than a lazy_static Mutex Option :)

@mitsuhiko mitsuhiko merged commit 46e3370 into master Feb 25, 2019
@mitsuhiko mitsuhiko deleted the feature/curl-pool branch February 25, 2019 08:07
szokeasaurusrex added a commit that referenced this pull request Feb 20, 2024
We removed Api::reset way back in #489. We should delete this comment that continues to reference Api::reset, since it is clearly inaccurate.
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.

2 participants