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

mem leak fix #3794

Merged
merged 8 commits into from Jul 8, 2022
Merged

mem leak fix #3794

merged 8 commits into from Jul 8, 2022

Conversation

gearama
Copy link
Member

@gearama gearama commented Jul 5, 2022

Mem leak due to raw pointer used for curl requests. The pointer if leaked when throwing from the subsequent checks.
Since we were wraping it in a unique pointer of CurlConnection whom's destructor takes care of cleaning up the curl pointer, right before returning, make sure the unique pointer gets created before the first potential throw.

Pull Request Checklist

Please leverage this checklist as a reminder to address commonly occurring feedback when submitting a pull request to make sure your PR can be reviewed quickly:

See the detailed list in the contributing guide.

  • C++ Guidelines
  • Doxygen docs
  • Unit tests
  • No unwanted commits/changes
  • Descriptive title/description
    • PR is single purpose
    • Related issue listed
  • [x]Comments in source
  • No typos
  • Update changelog
  • Not work-in-progress
  • External references or docs updated
  • Self review of PR done
  • Any breaking changes?

Copy link
Member

@vhvb1989 vhvb1989 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gloriously amazing!! thank you 🌮

Copy link
Member

@ahsonkhan ahsonkhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

sdk/core/azure-core/src/http/curl/curl.cpp Outdated Show resolved Hide resolved
sdk/core/azure-core/src/http/curl/curl.cpp Outdated Show resolved Hide resolved
sdk/core/azure-core/src/http/curl/curl.cpp Outdated Show resolved Hide resolved
@gearama gearama merged commit 46eaa38 into Azure:main Jul 8, 2022
azure-sdk added a commit to azure-sdk/vcpkg that referenced this pull request Aug 4, 2022
## 1.7.1 (2022-08-04)

### Bugs Fixed

- [[microsoft#3794]](Azure/azure-sdk-for-cpp#3794) Fix memory leak in `CurlTransport`.
- [[microsoft#3692]](Azure/azure-sdk-for-cpp#3692) Interrupted poll calls cause spurious HTTP request failures. (A community contribution, courtesy of _[johnwheffner](https://github.com/johnwheffner)_)

### Acknowledgments

Thank you to our developer community members who helped to make Azure Core better with their contributions to this release:

- John Heffner _([GitHub](https://github.com/johnwheffner))_
sthagen pushed a commit to sthagen/microsoft-vcpkg that referenced this pull request Aug 4, 2022
## 1.7.1 (2022-08-04)

### Bugs Fixed

- [[microsoft#3794]](Azure/azure-sdk-for-cpp#3794) Fix memory leak in `CurlTransport`.
- [[microsoft#3692]](Azure/azure-sdk-for-cpp#3692) Interrupted poll calls cause spurious HTTP request failures. (A community contribution, courtesy of _[johnwheffner](https://github.com/johnwheffner)_)

### Acknowledgments

Thank you to our developer community members who helped to make Azure Core better with their contributions to this release:

- John Heffner _([GitHub](https://github.com/johnwheffner))_
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

5 participants