Skip to content

Conversation

@ianton-ru
Copy link

Changelog category (leave one):

  • Critical Bug Fix (crash, data loss, RBAC) or LOGICAL_ERROR

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Check multiplexed connection before usage

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • Tiered Storage (2h)

@github-actions
Copy link

github-actions bot commented Oct 24, 2025

Workflow [PR], commit [112b8df]

state.connection = nullptr;
state.pool_entry = IConnectionPool::Entry();
--active_connection_count;
if (current_connection == old_connection)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it Ok that we don't acquire cancel_mutex here?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, invalidateReplica is called under that mutex already, like in MultiplexedConnections::disconnect https://github.com/Altinity/ClickHouse/blob/antalya/src/Client/MultiplexedConnections.cpp#L239

@Enmk
Copy link
Member

Enmk commented Oct 30, 2025

@ianton-ru please provide a proper description WHY we need this change and/or reference to the bug that it fixes. Otherwise there is no way to perform a code review of proper quality

@ianton-ru
Copy link
Author

@ianton-ru please provide a proper description WHY we need this change and/or reference to the bug that it fixes. Otherwise there is no way to perform a code review of proper quality

Crush catched by @filimonov.
Crush can be, when called disconenct method by some reason, in this case connection can be closed in invalidateReplica, and later pointer used when connection already deleted.

Copy link
Member

@Enmk Enmk left a comment

Choose a reason for hiding this comment

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

LGTM

@Enmk Enmk merged commit d122cd1 into antalya-25.8 Nov 7, 2025
132 of 137 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants