From 4e315e011061f3a345e0025a22c80c0ce13a12a3 Mon Sep 17 00:00:00 2001 From: ejona Date: Thu, 28 Aug 2014 14:21:40 -0700 Subject: [PATCH] Fix more transport error and shutdown scenarios. Previously, if the active transport had failed but there were still other running transports, we would claim to be shutdown before we actually were (and would end up calling notifyStopped twice). Also, if a transport immediately failed during startup, we could end up setting activeTransport to the already-failed transport, which meant we would perpetually try to use that broken transport. ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=74339777 --- .../com/google/net/stubby/ChannelImpl.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/com/google/net/stubby/ChannelImpl.java b/core/src/main/java/com/google/net/stubby/ChannelImpl.java index 9fbba41fe9..b8e3bd7685 100644 --- a/core/src/main/java/com/google/net/stubby/ChannelImpl.java +++ b/core/src/main/java/com/google/net/stubby/ChannelImpl.java @@ -52,12 +52,14 @@ public final class ChannelImpl extends AbstractService implements Channel { @Override protected synchronized void doStop() { - if (activeTransport != null) { - activeTransport.stopAsync(); - activeTransport = null; - // The last TransportListener will call notifyStopped(). - } else { + if (transports.isEmpty()) { notifyStopped(); + } else { + // The last TransportListener will call notifyStopped(). + if (activeTransport != null) { + activeTransport.stopAsync(); + activeTransport = null; + } } } @@ -72,13 +74,14 @@ public final class ChannelImpl extends AbstractService implements Channel { throw new IllegalStateException("Not running"); } ClientTransport newTransport = transportFactory.newClientTransport(); + activeTransport = newTransport; + transports.add(newTransport); + // activeTransport reference can be changed during calls to the transport, even if we hold the + // lock, due to reentrancy. newTransport.addListener( new TransportListener(newTransport), MoreExecutors.directExecutor()); - transports.add(newTransport); - // activeTransport reference can be changed during this call, even if we hold the lock, due to - // reentrancy. newTransport.startAsync(); - activeTransport = newTransport; + return newTransport; } return activeTransport; }