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.

Sign up for GitHub

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

Jump to bottom

[Proxy] Prevent leak of unreleased lookupRequestSemaphore permits #13812

Merged
merged 2 commits into from
Jan 19, 2022
Merged

[Proxy] Prevent leak of unreleased lookupRequestSemaphore permits #13812

merged 2 commits into from
Jan 19, 2022

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Jan 18, 2022 •
edited
Loading

Motivation

  • permits could leak if an exception happens
    • permits should be released in a try-finally block

Modifications

  • use try-finally to release permits
  • refactor code
    • reduce duplication
    • increase encapsulation (get rid of this.service.getLookupRequestSemaphore())

@lhotari lhotari added area/proxy doc-not-needed Your PR changes do not impact docs labels Jan 18, 2022
@lhotari lhotari self-assigned this Jan 18, 2022
eolivelli
eolivelli approved these changes Jan 18, 2022
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

nicoloboschi
nicoloboschi approved these changes Jan 18, 2022
View reviewed changes
Copy link
Contributor

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

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

good catch

@codelipenghui codelipenghui added this to the 2.10.0 milestone Jan 18, 2022
@lhotari lhotari merged commit 85b62e0 into apache:master Jan 19, 2022
codelipenghui pushed a commit that referenced this pull request Jan 21, 2022
@lhotari @codelipenghui
[Proxy] Prevent leak of unreleased lookupRequestSemaphore permits (#1…
d3a2ee4 
…3812)

* [Proxy] Prevent leak of unreleased lookupRequestSemaphore permits

- should release permit in try-finally block

* Cleanup code in LookupProxyHandler

(cherry picked from commit 85b62e0)
@codelipenghui codelipenghui added cherry-picked/branch-2.9 Archived: 2.9 is end of life and removed release/2.8.3 labels Jan 21, 2022
lhotari added a commit to datastax/pulsar that referenced this pull request May 4, 2022
@lhotari
[Proxy] Prevent leak of unreleased lookupRequestSemaphore permits (ap…
dcc07e8 
…ache#13812)

* [Proxy] Prevent leak of unreleased lookupRequestSemaphore permits

- should release permit in try-finally block

* Cleanup code in LookupProxyHandler

(cherry picked from commit 85b62e0)
lhotari added a commit to datastax/pulsar that referenced this pull request May 4, 2022
@lhotari
[Proxy] Prevent leak of unreleased lookupRequestSemaphore permits (ap…
50be626 
…ache#13812)

* [Proxy] Prevent leak of unreleased lookupRequestSemaphore permits

- should release permit in try-finally block

* Cleanup code in LookupProxyHandler

(cherry picked from commit 85b62e0)
lhotari added a commit to datastax/pulsar that referenced this pull request May 4, 2022
@lhotari
[Proxy] Prevent leak of unreleased lookupRequestSemaphore permits (ap…
80eb8e4 
…ache#13812)

* [Proxy] Prevent leak of unreleased lookupRequestSemaphore permits

- should release permit in try-finally block

* Cleanup code in LookupProxyHandler

(cherry picked from commit 85b62e0)
lhotari added a commit that referenced this pull request Jun 1, 2022
@lhotari
[Proxy] Prevent leak of unreleased lookupRequestSemaphore permits (#1…
dbe0518 
…3812)

* [Proxy] Prevent leak of unreleased lookupRequestSemaphore permits

- should release permit in try-finally block

* Cleanup code in LookupProxyHandler

(cherry picked from commit 85b62e0)
(cherry picked from commit dcc07e8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicoloboschi nicoloboschi nicoloboschi approved these changes

@shoothzj shoothzj shoothzj approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@eolivelli eolivelli eolivelli approved these changes

@merlimat merlimat merlimat approved these changes

@315157973 315157973 Awaiting requested review from 315157973

@congbobo184 congbobo184 Awaiting requested review from congbobo184

@michaeljmarshall michaeljmarshall Awaiting requested review from michaeljmarshall

Assignees

@lhotari lhotari

Labels
area/proxy cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.9.2
Projects
None yet
Milestone
2.10.0
Development

Successfully merging this pull request may close these issues.
6 participants
@lhotari @merlimat @eolivelli @codelipenghui @shoothzj @nicoloboschi