Skip to content

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

Merged
carl-mastrangelo merged 2 commits intogrpc:masterfrom
carl-mastrangelo:servdoc
Jun 20, 2019
Merged

core,netty: block server shutdown until the socket is unbound#5905
carl-mastrangelo merged 2 commits intogrpc:masterfrom
carl-mastrangelo:servdoc

Conversation

@carl-mastrangelo
Copy link
Copy Markdown
Contributor

No description provided.

@carl-mastrangelo carl-mastrangelo requested a review from ejona86 June 19, 2019 23:16

/**
* 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
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

eventLoopReferenceCounter.release();
}
});
if (Thread.currentThread().isInterrupted()) {
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.

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
Copy Markdown
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.

2 participants