mirror of https://github.com/grpc/grpc-java.git
TransportSet shutdown() also shuts down the pending transport.
Previously TransportSet.shutdown() only shuts down the active transport, which means a transport will not be shutdown if it's not ready yet. This issue was introduced by #1494 that postponed the assignment of the active transport till transport ready.
This commit is contained in:
parent
9f37951680
commit
c08d74fec4
|
|
@ -102,6 +102,13 @@ final class TransportSet implements WithLogId {
|
||||||
private final Collection<ManagedClientTransport> transports =
|
private final Collection<ManagedClientTransport> transports =
|
||||||
new ArrayList<ManagedClientTransport>();
|
new ArrayList<ManagedClientTransport>();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The to-be active transport, which is not ready yet.
|
||||||
|
*/
|
||||||
|
@GuardedBy("lock")
|
||||||
|
@Nullable
|
||||||
|
private ManagedClientTransport pendingTransport;
|
||||||
|
|
||||||
private final LoadBalancer<ClientTransport> loadBalancer;
|
private final LoadBalancer<ClientTransport> loadBalancer;
|
||||||
|
|
||||||
@GuardedBy("lock")
|
@GuardedBy("lock")
|
||||||
|
|
@ -194,6 +201,7 @@ final class TransportSet implements WithLogId {
|
||||||
log.log(Level.FINE, "[{0}] Created {1} for {2}",
|
log.log(Level.FINE, "[{0}] Created {1} for {2}",
|
||||||
new Object[] {getLogId(), transport.getLogId(), address});
|
new Object[] {getLogId(), transport.getLogId(), address});
|
||||||
}
|
}
|
||||||
|
pendingTransport = transport;
|
||||||
transports.add(transport);
|
transports.add(transport);
|
||||||
transport.start(new TransportListener(transport, delayedTransport, address));
|
transport.start(new TransportListener(transport, delayedTransport, address));
|
||||||
}
|
}
|
||||||
|
|
@ -257,6 +265,7 @@ final class TransportSet implements WithLogId {
|
||||||
*/
|
*/
|
||||||
final void shutdown() {
|
final void shutdown() {
|
||||||
ManagedClientTransport savedActiveTransport;
|
ManagedClientTransport savedActiveTransport;
|
||||||
|
ManagedClientTransport savedPendingTransport;
|
||||||
boolean runCallback = false;
|
boolean runCallback = false;
|
||||||
synchronized (lock) {
|
synchronized (lock) {
|
||||||
if (shutdown) {
|
if (shutdown) {
|
||||||
|
|
@ -265,6 +274,7 @@ final class TransportSet implements WithLogId {
|
||||||
// Transition to SHUTDOWN
|
// Transition to SHUTDOWN
|
||||||
shutdown = true;
|
shutdown = true;
|
||||||
savedActiveTransport = activeTransport;
|
savedActiveTransport = activeTransport;
|
||||||
|
savedPendingTransport = pendingTransport;
|
||||||
activeTransport = null;
|
activeTransport = null;
|
||||||
if (transports.isEmpty()) {
|
if (transports.isEmpty()) {
|
||||||
runCallback = true;
|
runCallback = true;
|
||||||
|
|
@ -274,6 +284,9 @@ final class TransportSet implements WithLogId {
|
||||||
if (savedActiveTransport != null) {
|
if (savedActiveTransport != null) {
|
||||||
savedActiveTransport.shutdown();
|
savedActiveTransport.shutdown();
|
||||||
}
|
}
|
||||||
|
if (savedPendingTransport != null) {
|
||||||
|
savedPendingTransport.shutdown();
|
||||||
|
}
|
||||||
if (runCallback) {
|
if (runCallback) {
|
||||||
callback.onTerminated();
|
callback.onTerminated();
|
||||||
}
|
}
|
||||||
|
|
@ -358,7 +371,9 @@ final class TransportSet implements WithLogId {
|
||||||
"Unexpected non-null activeTransport");
|
"Unexpected non-null activeTransport");
|
||||||
} else if (activeTransport == delayedTransport) {
|
} else if (activeTransport == delayedTransport) {
|
||||||
// Transition to READY
|
// Transition to READY
|
||||||
|
Preconditions.checkState(pendingTransport == transport, "transport mismatch");
|
||||||
activeTransport = transport;
|
activeTransport = transport;
|
||||||
|
pendingTransport = null;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
delayedTransport.setTransport(transport);
|
delayedTransport.setTransport(transport);
|
||||||
|
|
|
||||||
|
|
@ -498,6 +498,22 @@ public class TransportSetTest {
|
||||||
assertEquals(0, transports.size());
|
assertEquals(0, transports.size());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void shutdownBeforeTransportReady() throws Exception {
|
||||||
|
SocketAddress addr = mock(SocketAddress.class);
|
||||||
|
createTransportSet(addr);
|
||||||
|
|
||||||
|
ClientTransport pick = transportSet.obtainActiveTransport();
|
||||||
|
MockClientTransportInfo transportInfo = transports.poll();
|
||||||
|
assertNotSame(transportInfo.transport, pick);
|
||||||
|
|
||||||
|
// Shutdown the TransportSet before the pending transport is ready
|
||||||
|
transportSet.shutdown();
|
||||||
|
|
||||||
|
// The transport should've been shut down even though it's not the active transport yet.
|
||||||
|
verify(transportInfo.transport).shutdown();
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void obtainTransportAfterShutdown() throws Exception {
|
public void obtainTransportAfterShutdown() throws Exception {
|
||||||
SocketAddress addr = mock(SocketAddress.class);
|
SocketAddress addr = mock(SocketAddress.class);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue