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

Avoid infinite waiting for consumer close #11347

Merged
merged 1 commit into from
Jul 22, 2021
Merged

Avoid infinite waiting for consumer close #11347

merged 1 commit into from
Jul 22, 2021

Conversation

shoothzj
Copy link
Member

Motivation

If there are two Events

  • EventA: Server close the consumer
  • EventB: consumer close self

If EventA and EventB concurrently, then the cnx.ctx() has probability to be null. Which occurs a NPE, lead to infinite wait future

Modifications

If the cnx.ctx() is already null, we ignore the exception too.

@Anonymitaet
Copy link
Member

Anonymitaet commented Jul 19, 2021 •
edited
Loading

@shoothzj thanks for your contribution. For this PR, do we need to update docs?

(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@eolivelli eolivelli added this to the 2.9.0 milestone Jul 19, 2021
@eolivelli
Copy link
Contributor

thanks for addressing my comments.

did you see this problem in production or do you have a stacktrace with an NPE to show ?

@shoothzj
Copy link
Member Author

thanks for addressing my comments.

did you see this problem in production or do you have a stacktrace with an NPE to show ?

Yes, I noticed the problem in prodcution. The client calls unsubscribe and close linely(or continously, orderly, sorry for my english).
The unsubsribe success on server side, but failed on client side. Then the server call client to close, client change ctx to null. But for unsubscribe failed on client side, the client state is not closed or closing, consumer begin to close, but the ctx is already null, NPE occurred.

@shoothzj shoothzj requested a review from eolivelli July 20, 2021 07:05
eolivelli
eolivelli approved these changes Jul 20, 2021
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.

LGTM

@lhotari @codelipenghui @congbobo184 PTAL

@shoothzj
Copy link
Member Author

@shoothzj thanks for your contribution. For this PR, do we need to update docs?

(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

I think we don't need any docs.

@Anonymitaet Anonymitaet added the doc-not-needed Your PR changes do not impact docs label Jul 22, 2021
@Anonymitaet
Copy link
Member

@shoothzj OK. Can you leave doc-related info for future PRs? Thanks

@sijie sijie merged commit 4d3fdae into apache:master Jul 22, 2021
Technoboy- pushed a commit to Technoboy-/pulsar that referenced this pull request Jul 22, 2021
@shoothzj @Technoboy-
Avoid infinite waiting for consumer close (apache#11347)
47dfdd9 
### Motivation
If there are two Events
- EventA: Server close the consumer
- EventB: consumer close self

If EventA and EventB concurrently, then the `cnx.ctx()` has probability to be null. Which occurs a NPE, lead to infinite wait future

### Modifications

If the `cnx.ctx()` is already null, we ignore the exception too.
codelipenghui pushed a commit that referenced this pull request Jul 23, 2021
@shoothzj @codelipenghui
Avoid infinite waiting for consumer close (#11347)
a3f179c 
### Motivation
If there are two Events
- EventA: Server close the consumer
- EventB: consumer close self

If EventA and EventB concurrently, then the `cnx.ctx()` has probability to be null. Which occurs a NPE, lead to infinite wait future

### Modifications

If the `cnx.ctx()` is already null, we ignore the exception too.

(cherry picked from commit 4d3fdae)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Jul 23, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
@shoothzj @ciaocloud
Avoid infinite waiting for consumer close (apache#11347)
9090cf4 
### Motivation
If there are two Events
- EventA: Server close the consumer
- EventB: consumer close self

If EventA and EventB concurrently, then the `cnx.ctx()` has probability to be null. Which occurs a NPE, lead to infinite wait future

### Modifications

If the `cnx.ctx()` is already null, we ignore the exception too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sijie sijie sijie approved these changes

@eolivelli eolivelli eolivelli approved these changes

@lhotari lhotari Awaiting requested review from lhotari

@codelipenghui codelipenghui Awaiting requested review from codelipenghui

Assignees

@shoothzj shoothzj

Labels
area/client cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.1
Projects
None yet
Milestone
2.9.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@shoothzj @Anonymitaet @eolivelli @sijie @codelipenghui