Skip to content

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Sep 27, 2025

Motivation

LookupService is replaced by calling reloadLookUp. However the previous instance is not closed. This causes a thread leak.

Modifications

Call the close method before replacing LookupService.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari added this to the 4.2.0 milestone Sep 27, 2025
@lhotari lhotari self-assigned this Sep 27, 2025
@lhotari lhotari added type/bug The PR fixed a bug or issue reported a bug area/client ready-to-test labels Sep 27, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 27, 2025
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.18%. Comparing base (f7a02aa) to head (43f38fc).

Files with missing lines Patch % Lines
...rg/apache/pulsar/client/impl/PulsarClientImpl.java 40.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24794      +/-   ##
============================================
- Coverage     74.27%   74.18%   -0.09%     
+ Complexity    33676    33295     -381     
============================================
  Files          1907     1907              
  Lines        148703   148708       +5     
  Branches      17249    17249              
============================================
- Hits         110443   110324     -119     
- Misses        29439    29569     +130     
+ Partials       8821     8815       -6     
Flag Coverage Δ
inttests 26.34% <0.00%> (-0.23%) ⬇️
systests 22.75% <0.00%> (-0.20%) ⬇️
unittests 73.71% <40.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rg/apache/pulsar/client/impl/PulsarClientImpl.java 75.75% <40.00%> (-0.29%) ⬇️

... and 82 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

}

public void reloadLookUp() throws PulsarClientException {
if (lookup != null) {
Copy link
Member

Choose a reason for hiding this comment

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

@lhotari The lookup variable is not declared as volatile, which may lead to a visibility issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/client doc-not-needed Your PR changes do not impact docs ready-to-test type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants