Skip to content

[fix][client] Avoid closing reused ServiceUrlProvider in another PulsarClientImpl instance#25947

Merged
merlimat merged 1 commit into
apache:masterfrom
oneby-wang:service-url-provider-lifecycle
Jun 7, 2026
Merged

[fix][client] Avoid closing reused ServiceUrlProvider in another PulsarClientImpl instance#25947
merlimat merged 1 commit into
apache:masterfrom
oneby-wang:service-url-provider-lifecycle

Conversation

@oneby-wang

@oneby-wang oneby-wang commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Motivation

When the same ServiceUrlProvider instance is reused to build a second PulsarClientImpl, the provider rejects the second initialization because each provider instance is tied to one client lifecycle.

The second client constructor currently enters the failure cleanup path and calls shutdown(), which closes conf.getServiceUrlProvider() unconditionally. This can close a provider that is still owned by the first live client.

Modifications

  • Track whether the current PulsarClientImpl successfully initialized its ServiceUrlProvider.
  • Close the provider during shutdown() only when this client successfully initialized it.
  • Add a regression test that verifies a failed second client build does not close the provider used by the first client.

Verifying this change

  • Make sure that the change passes the CI checks.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

@void-ptr974 void-ptr974 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.

LGTM. The fix correctly ties ServiceUrlProvider.close() to successful initialization by the current client, and the added regression test covers the reused-provider failure path.

@merlimat merlimat merged commit 4bcc4a0 into apache:master Jun 7, 2026
44 of 45 checks passed
@lhotari lhotari added this to the 5.0.0-M1 milestone Jun 12, 2026
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.

4 participants