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

Avoid any chance at NPE in RealWebSocket #6047

Merged
merged 1 commit into from May 13, 2020
Merged

Conversation

yschimke
Copy link
Collaborator

@yschimke yschimke commented May 12, 2020

Pretty sure that with correct locking that this already can't be null. The syncrhonized memory barrier in writePingFrame, which happens via the task runner, should always be after the syncrhonized memory barrier in initReaderAndWriter.

But this subtle change should be a noop but delay the ping if we find ourselves somehow with null.

To address #5705

@yschimke
Copy link
Collaborator Author

@dave-r12 You are awesome at these puzzlers, riddle me this, how was this ever null?

Copy link
Member

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

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

Neat. Yep, I think this is null if we race onReadClose().

@swankjesse swankjesse merged commit 836c2f9 into square:master May 13, 2020
@yschimke yschimke deleted the ping branch May 21, 2020 20:21
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.

None yet

2 participants