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

core,netty: block server shutdown until the socket is unbound #5905

Merged
merged 2 commits into from
Jun 20, 2019

Conversation

carl-mastrangelo
Copy link
Contributor

No description provided.

@@ -102,6 +104,8 @@ public int getPort() {

/**
* Initiates an orderly shutdown in which preexisting calls continue but new calls are rejected.
* After calling this method, clients may no longer connect to the listening socket(s).
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I feel like this first sentence is mostly covered by the first. And the user couldn't really notice if we accepted another connection anyway. How about "After this call returns, this server has released the listening socket(s) and it may be reused by another server."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -283,6 +283,15 @@ public void operationComplete(ChannelFuture future) throws Exception {
eventLoopReferenceCounter.release();
}
});
if (Thread.currentThread().isInterrupted()) {
Copy link
Member

Choose a reason for hiding this comment

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

Why this condition? Is it an optimization? Honestly, I'd prefer not to optimize this case, which also has benefits like the FINE logging happening independent of how interrupt races.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC normal futures throw if they try to block while the thread is interrupted, but I wasn't sure Netty's would do the same. I'm happy to remove it

@carl-mastrangelo carl-mastrangelo merged commit 74e945c into grpc:master Jun 20, 2019
@carl-mastrangelo carl-mastrangelo deleted the servdoc branch June 20, 2019 00:23
@carl-mastrangelo carl-mastrangelo restored the servdoc branch August 17, 2019 01:11
@lock lock bot locked as resolved and limited conversation to collaborators Nov 15, 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.

None yet

2 participants