Skip to content

stub: avoid race in onHalfClose server StreamObserver#5991

Merged
carl-mastrangelo merged 1 commit intogrpc:masterfrom
carl-mastrangelo:rayce
Jul 19, 2019
Merged

stub: avoid race in onHalfClose server StreamObserver#5991
carl-mastrangelo merged 1 commit intogrpc:masterfrom
carl-mastrangelo:rayce

Conversation

@carl-mastrangelo
Copy link
Copy Markdown
Contributor

No description provided.

@zhangkun83
Copy link
Copy Markdown
Contributor

LGTM

request = null;
responseObserver.freeze();
if (call.isReady()) {
if (!wasReady) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is inverted. It should be if (wasReady)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, I thought you said onReady should only be called once, and it was which ever method (onHalfClose or onReady) was invoked first. Is that not correct?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Before onHalfClose(), onReadyHandler == null. So if there was an onReady callback, the code "ignored" it. The onReady() callback will generally happen before onHalfClose(). This check was to "replay" that missed onReady call.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see, updated.

@carl-mastrangelo carl-mastrangelo merged commit 148b7de into grpc:master Jul 19, 2019
@carl-mastrangelo carl-mastrangelo deleted the rayce branch July 19, 2019 19:24
@lock lock bot locked as resolved and limited conversation to collaborators Oct 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants