Skip to content

CCDBApi: avoid leaking curl handle.#13061

Merged
martenole merged 1 commit into
AliceO2Group:devfrom
ktf:pr13061
Apr 23, 2024
Merged

CCDBApi: avoid leaking curl handle.#13061
martenole merged 1 commit into
AliceO2Group:devfrom
ktf:pr13061

Conversation

@ktf

@ktf ktf commented Apr 22, 2024

Copy link
Copy Markdown
Member

CCDBApi: avoid leaking curl handle.

@ktf ktf requested review from a team, Barthelemy, costing and sawenzel as code owners April 22, 2024 10:37
@github-actions

Copy link
Copy Markdown
Contributor

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2023-pbpb-apass3
async-2023-pbpb-apass4
async-2022-pp-apass6-2023-PbPb-apass2
async-2022-pp-apass4
async-2022-pp-apass4-accepted
async-2022-pp-apass6-2023-PbPb-apass2-accepted
async-2023-pbpb-apass3-accepted
async-2023-pbpb-apass4-accepted

costing
costing previously approved these changes Apr 22, 2024
@martenole

Copy link
Copy Markdown
Contributor

Hi @ktf
thanks, this looks very good and if I run the FST with it the leak rate is strongly reduced. It seems the build checks are failing though, could you check please?

/sw/SOURCES/O2/13061-slc7_x86-64/0/CCDB/test/testCcdbApiDownloader.cxx(173): �[1;31;49merror: in "asynch_schedule_test": check httpCode == 200 has failed�[0;39;49m

@ktf

ktf commented Apr 22, 2024

Copy link
Copy Markdown
Member Author

I frankly think the test is wrong, but indeed it needs understanding. @costing could you cc Michal? I do not recall his handle.

@ktf

ktf commented Apr 22, 2024

Copy link
Copy Markdown
Member Author

Also the rest of the leak comes from the HTTPHEADER slist, however the way the code is currenlty structured, it's not easy to clean it up correctly.

@costing

costing commented Apr 22, 2024

Copy link
Copy Markdown
Collaborator

Adding @TrifleMichael

@ktf

ktf commented Apr 22, 2024

Copy link
Copy Markdown
Member Author

Updated with the fix for the remaining leak. I think the delete in the test should not be there, but someone should probably doublecheck the logic. With this, my reproducer does not show any leak whatsoever.

@martenole martenole left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. I don't understand why the formatting check fails

@martenole martenole merged commit 35d6be7 into AliceO2Group:dev Apr 23, 2024
@ktf

ktf commented Apr 23, 2024

Copy link
Copy Markdown
Member Author

I think it's some glitch on the github side, actually. The code formats fine locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants