Example HelloWorldServer does not gracefully shutdown

[dounan]

What version of gRPC-Java are you using?

On a fork up at commit d168632

What is your environment?

Mac OSX 10.14.6
Java 11.0.2

What did you expect to see?

The server should allow in-flight RPCs to finish

What did you see instead?

The client receives error WARNING: RPC 1 failed: Status{code=UNAVAILABLE, description=Network closed for unknown reason, cause=null}

Steps to reproduce the bug

• Add a call to Thread.sleep(15000) in io.grpc.examples.helloworld.HelloWorldServer.GreeterImpl.sayHello
• Build and run the server
• Run the client
• Ctrl+C the server once it gets the request

It seems like server.shutdown() does not wait for in-flight RPCs to finish before returning. The documentation makes it seem like that it should block.

I am able to "fix" the issue by adding a call to server.awaitTermination() in the stop method.

[sanjaypujare]

This is behaving as designed/expected. The doc for server.shutdown() does say "..preexisting calls continue..." but does not imply that shutdown() blocks until the calls end. So shutdown will return after initiating orderly shutdown but you need to wait using server.awaitTermination() for the inflight calls to finish. If the doc seems unclear/ambiguous suggest new wording.

[dounan]

Would it make sense to update all the example code servers to reflect the fact that awaitTermination() needs to be called? Both that and the wording on the documentation is a bit misleading. I can try putting together a PR for that change

[sanjaypujare]

Would it make sense to update all the example code servers to reflect the fact that awaitTermination() needs to be called?

Seems like a good idea since awaitTermination() is currently not called in case of control-C termination (shutdown-hook).

Both that and the wording on the documentation is a bit misleading. I can try putting together a PR for that change

Even if not misleading, more clarification cannot hurt.

[dounan]

Created a PR here: #6512

[ejona86]

Taken from #6512:
Hmmm... something else is going on. The blockUntilShutdown() in the main() should be enough...

On ctrl-c, the JVM simply calls start() on the threads registered, and nothing really more. What causes JVM shutdown is when there are no non-daemon threads. The main thread is non-daemon, so it should keep the JVM alive.

---

I'm going to try to reproduce and see if I can find the root cause. The problem may be that the server is considering itself terminated prematurely. It might also be some of the subtleties with how RST vs graceful close is triggered (see SO_LINGER documentation for a small glimpse).

[dounan]

Thanks @ejona86! Interested to see what you find out

[ejona86]

Hmm... it appears my understand was wrong. Based on the addShutdownHook() documentation, after a Ctrl-C the jvm will exit after all the hooks are run, and it seems to imply it will ignore any non-daemon threads. I guess that makes sense, since the jvm will exit on Ctrl-c if you haven't registered any shutdown hooks.

That does disagree with the Thread documentation, since it only lists two ways (System.exit() and no more non-daemon threads). But I guess some language lawyering would say that doesn't apply in termination scenarios.

[dounan]

The Thread documentation is a bit vague. I wonder if it's because SIGINT actually triggers the Runtime.exit() call that the documentation refers to

[sanjaypujare]

The Thread documentation is a bit vague. I wonder if it's because SIGINT actually triggers the Runtime.exit() call that the documentation refers to

exit signifies proper exit (from the code) and cannot be triggered by SIGINT AFAIK

[ejona86]

exit() in Java != exit() in C; it does a lot behind the scenes in Java. I did feel like SIGINT behaves similar to exit() having been called. But the documentation considers ctrl-C separate from exit(), but then calls out the exit() case as a special case, but that special case appears to apply to ctrl-c even though the documentation doesn't say it applies to ctrl-c.

Whatever.

[dounan]

🤡