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.

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

Fix nil point dereference in notifyEvent #1050

Closed
wants to merge 1 commit into from
Closed

Fix nil point dereference in notifyEvent #1050

wants to merge 1 commit into from

Conversation

chenchun
Copy link
Contributor

Signed-off-by: Chun Chen ramichen@tencent.com

Don't know how the scenarios mentioned in #985 happened.
But I think we should prevent daemon crash.

Signed-off-by: Chun Chen <ramichen@tencent.com>
@mavenugo
Copy link
Contributor

thanks @chenchun . am always supportive of preventive checks. But @mrjana had some good reasons to leave some of these unprotected. Though it is safe to do nil checks, it would be good to understand the scenario which might have triggered it. @mrjana do you have any suggestions ?

@mavenugo
Copy link
Contributor

@chenchun I did some more investigation and a defensive check in this case will hide the issue rather than solving the root-cause. The correct fix is to make sure the leave processing happen even if the delete endpoint takes place before the Serf gets a chance to distribute the event to the neighbors.

I am working on the proper fix for this. We can close this PR if we are all happy with my upcoming patch.

@chenchun
Copy link
Contributor Author

@mavenugo Thanks. Your fix makes sense.

@chenchun chenchun closed this Mar 30, 2016
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.

3 participants