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

fix: do not use CURL_SHARE features #3860

Merged
merged 2 commits into from
Apr 20, 2020

Conversation

coryan
Copy link
Member

@coryan coryan commented Apr 20, 2020

It seems safer to stop using the CURL_SHARE features until
curl/curl#4915 is fixed. We have outstanding bug reports of crashes in
multi-threaded applications, and our own tests trigger errors when
runing under ThreadSanitizer. I considered making this an optional
feature, disabled by default, but the only case where this could be
useful is a single-threaded application that starts multiple downloads
at the same time, and the benefits in that case are (probably) very
modest: the connection setup would be slightly faster, but everything
else would be the same.

Part of the work for #3832

Updated the API baseline, all the changes are in the
::google::cloud::storage::internal namespace.


This change is Reviewable

It seems safer to stop using the CURL_SHARE features until
curl/curl#4915 is fixed. We have outstanding bug reports of crashes in
multi-threaded applications, and our own tests trigger errors when
runing under ThreadSanitizer. I considered making this an optional
feature, disabled by default, but the only case where this could be
useful is a single-threaded application that starts multiple downloads
at the same time, and the benefits in that case are (probably) very
modest: the connection setup would be slightly faster, but everything
else would be the same.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 20, 2020
@codecov
Copy link

codecov bot commented Apr 20, 2020

Codecov Report

Merging #3860 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3860      +/-   ##
==========================================
- Coverage   92.49%   92.48%   -0.02%     
==========================================
  Files         504      504              
  Lines       43190    43152      -38     
==========================================
- Hits        39949    39907      -42     
- Misses       3241     3245       +4     
Impacted Files Coverage Δ
google/cloud/storage/internal/curl_client.cc 96.73% <ø> (+1.69%) ⬆️
google/cloud/storage/internal/curl_client.h 100.00% <ø> (ø)
google/cloud/bigtable/rpc_backoff_policy.h 60.00% <0.00%> (-40.00%) ⬇️
...gle/cloud/bigtable/testing/inprocess_data_client.h 83.33% <0.00%> (-16.67%) ⬇️
...oogle/cloud/bigtable/internal/unary_client_utils.h 84.37% <0.00%> (-15.63%) ⬇️
google/cloud/storage/well_known_headers.h 79.16% <0.00%> (-12.50%) ⬇️
google/cloud/storage/well_known_parameters.h 90.90% <0.00%> (-9.10%) ⬇️
google/cloud/storage/internal/hash_validator.h 80.00% <0.00%> (-8.89%) ⬇️
...gle/cloud/storage/internal/curl_request_builder.cc 92.30% <0.00%> (-3.85%) ⬇️
google/cloud/storage/internal/object_requests.h 96.92% <0.00%> (-3.08%) ⬇️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35348f0...5c49470. Read the comment docs.

@coryan coryan marked this pull request as ready for review April 20, 2020 03:24
@coryan coryan merged commit 2d6aaec into googleapis:master Apr 20, 2020
@coryan coryan deleted the remove-curlsh-feature branch April 20, 2020 10:21
frankyn pushed a commit to frankyn/google-cloud-cpp that referenced this pull request Apr 29, 2021
🤖 I have created a release \*beep\* \*boop\*
---
## [0.81.0](https://www.github.com/googleapis/google-cloud-go/compare/v0.80.0...v0.81.0) (2021-04-02)


### Features

* **datacatalog:** Policy Tag Manager v1 API service feat: new RenameTagTemplateFieldEnumValue API feat: adding fully_qualified_name in lookup and search feat: added DATAPROC_METASTORE integrated system along with new entry types: DATABASE and SERVICE docs: Documentation improvements ([2b02a03](https://www.github.com/googleapis/google-cloud-go/commit/2b02a03ff9f78884da5a8e7b64a336014c61bde7))
* **dialogflow/cx:** include original user query in WebhookRequest; add GetTextCaseresult API. doc: clarify resource format for session response. ([a0b1f6f](https://www.github.com/googleapis/google-cloud-go/commit/a0b1f6faae77d014fdee166ab018ddcd6f846ab4))
* **dialogflow/cx:** include original user query in WebhookRequest; add GetTextCaseresult API. doc: clarify resource format for session response. ([b5b4da6](https://www.github.com/googleapis/google-cloud-go/commit/b5b4da6952922440d03051f629f3166f731dfaa3))
* **dialogflow:** expose MP3_64_KBPS and MULAW for output audio encodings. ([b5b4da6](https://www.github.com/googleapis/google-cloud-go/commit/b5b4da6952922440d03051f629f3166f731dfaa3))
* **secretmanager:** Rotation for Secrets ([2b02a03](https://www.github.com/googleapis/google-cloud-go/commit/2b02a03ff9f78884da5a8e7b64a336014c61bde7))


### Bug Fixes

* **internal/godocfx:** filter out non-Cloud ([googleapis#3878](https://www.github.com/googleapis/google-cloud-go/issues/3878)) ([625aef9](https://www.github.com/googleapis/google-cloud-go/commit/625aef9b47181cf627587cc9cde9e400713c6678))

This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants