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

Cherry pick #50334 to 22.8: Fix crash when Pool::Entry::disconnect() is called #51044

Conversation

robot-clickhouse-ci-2
Copy link
Contributor

Original pull-request #50334

This pull-request is a first step of an automated backporting.
It contains changes like after calling a local command git cherry-pick.
If you intend to continue backporting this changes, then resolve all conflicts if any.
Otherwise, if you do not want to backport them, then just close this pull-request.

The check results does not matter at this step - you can safely ignore them.

Note

This pull-request will be merged automatically as it reaches the mergeable state, do not merge it manually.

If the PR was closed and then reopened

If it stuck, check #50334 for pr-backports-created and delete it if necessary. Manually merging will do nothing, since pr-backports-created prevents the original PR #50334 from being processed.

If you want to recreate the PR: delete the pr-cherrypick label and delete this branch.
You may also need to delete the pr-backports-created label from the original PR.

valbok and others added 2 commits May 30, 2023 10:29
Many Pool::Entry objects can keep the same pointer to Pool::Connection.
If Pool::Entry::disconnect() is called on one such object,
Pool::removeConnection() is called to remove Pool::Connection from the pool,
where connection->ref_count is cleared and connection->removed_from_pool is set.

Next Pool::Entry::~Entry() calls decrementRefCount() with

1. const auto ref_count = data->ref_count.fetch_sub(1);
   where data->ref_count will be negative, since it was cleared
2. checks removed_from_pool and deletes Pool::Connection
   but there might be multiple Entry objects still keep pointer to this Pool::Connection

Suggesting not to clear ref_count on disconnect()
and delete Pool::Connection only on the last Pool::Entry is being destroyed.

Fixes ea375ef
Fix crash when Pool::Entry::disconnect() is called
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added pr-cherrypick Cherry-pick of merge-commit before backporting. Do not use manually - automated use only! do not test disable testing on pull request labels Jun 15, 2023
@kssenii kssenii closed this Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not test disable testing on pull request pr-cherrypick Cherry-pick of merge-commit before backporting. Do not use manually - automated use only!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants