While starting managed channels on client we get error: Channel for target was not shutdown properly!!!

[nddipiazza]

What version of gRPC are you using?

1.8.0 (and we also saw this in 1.9.0)

We get this error message when starting up a client.

Jan 31, 2018 7:18:57 PM io.grpc.internal.ManagedChannelImpl$ManagedChannelReference cleanQueue
SEVERE: *~*~*~ Channel io.grpc.internal.ManagedChannelImpl-2096 for target 127.0.0.1:8771 was not shutdown properly!!! ~*~*~*
    Make sure to call shutdown()/shutdownNow() and awaitTermination().
java.lang.RuntimeException: ManagedChannel allocation site
    at io.grpc.internal.ManagedChannelImpl$ManagedChannelReference.<init>(ManagedChannelImpl.java:1007)
    at io.grpc.internal.ManagedChannelImpl.<init>(ManagedChannelImpl.java:430)
    at io.grpc.internal.AbstractManagedChannelImplBuilder.build(AbstractManagedChannelImplBuilder.java:337)
    at com.nick.DiscoveryChannelFactory.newChannelFromAddress(DiscoveryChannelFactory.java:171)

Is this because a previous channel failed to be shut down correctly and now we are starting up another one?

[carl-mastrangelo]

It means you didn't call shutdown() or shutdownNow() on your channel, and it was garbage collected.

[nddipiazza]

@carl-mastrangelo

can you help me understand how to cleanly clean up after channels?

so if i understand, when calling

channel = ManagedChannelBuilder
            .forTarget(address)
            .idleTimeout(5, TimeUnit.SECONDS)
            .usePlaintext(true)
            .executor(executorService)
            .build();

should we be wrapping this in a

Channel channel = null;
try {
  channel = ManagedChannelBuilder
            .forTarget(address)
            .idleTimeout(5, TimeUnit.SECONDS)
            .usePlaintext(true)
            .executor(executorService)
            .build();
} finally {
  if (channel != null) channel.shutdown();
}

??

[carl-mastrangelo]

You can do that, but it would be surprising if you only started a Channel for making one RPC. If that is your use case, then the snippet you propose is correct.

[nddipiazza]

@carl-mastrangelo so... i think i'm understanding this. if you create a channel, but then do not hang onto the reference, it will GC then this error is gonna spit out.

we didn't see this in 1.6.1. was this new?

did you add this to prevent connection leaks? make them errors versus silent failures?

confused why it just happened after upgrade from 1.6.1 to 1.8.0

[carl-mastrangelo]

@nddipiazza It was added I think after 1.6. Internally, we have a similar mechanism because it causes the connections to get torn down abruptly, which is bad from the server's perspective.

At least at Google, several teams have monitoring and metrics for unexpected errors which can cause alerts. When a channels is GC'd this causes those numbers to go up. Closing down cleanly denoises those numbers. (Also, in our case at least, pretty much every server is also a client, so everyone benefits if shutdown happens normally).