Spanner: Do not hand out a closed Spanner instance from SpannerOptions.getService()

google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java

@@ -105,4 +105,7 @@ public interface Spanner extends Service<SpannerOptions>, AutoCloseable {
    */
   @Override
   void close();
+
+  /** @return <code>true</code> if this {@link Spanner} object is closed. */
+  boolean isClosed();
 }

google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java

@@ -291,6 +291,11 @@ public void close() {
     }
   }
 
+  @Override
+  public boolean isClosed() {
+    return spannerIsClosed;
+  }
+
   /**
    * Checks that the current context is still valid, throwing a CANCELLED or DEADLINE_EXCEEDED error
    * if not.

google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java

@@ -298,6 +298,36 @@ protected SpannerRpc getSpannerRpcV1() {
     return (SpannerRpc) getRpc();
   }
 
+  /**
+   * Returns a {@link Spanner} service object. Note that only the first call to this method will
+   * create a new instance, and all subsequent calls to this method will return the same instance.
+   * Calling this method after the {@link Spanner} instance has been closed will cause an exception.
+   */
+  @Override
+  public Spanner getService() {
+    Spanner spanner = super.getService();
+    if (spanner.isClosed()) {
+      throw new IllegalStateException(
+          "The Spanner service of this SpannerOptions has been closed. Create a new SpannerOptions instance and call getService() on the new instance instead.");
+    }
+    return spanner;
+  }
+
+  /**
+   * Returns a {@link SpannerRpc} object. Note that only the first call to this method will create a
+   * new instance, and all subsequent calls to this method will return the same instance. Calling
+   * this method after the instance has been closed will cause an exception.
+   */
+  @Override
+  public ServiceRpc getRpc() {
+    SpannerRpc rpc = (SpannerRpc) super.getRpc();
+    if (rpc.isClosed()) {
+      throw new IllegalStateException(
+          "The Spanner service of this SpannerOptions has been closed. Create a new SpannerOptions instance and call getRpc() on the new instance instead.");
+    }
+    return rpc;
+  }
+
   @SuppressWarnings("unchecked")
   @Override
   public Builder toBuilder() {

google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java

@@ -168,6 +168,7 @@ private synchronized void shutdown() {
   private static final int DEFAULT_PERIOD_SECONDS = 10;
 
   private final ManagedInstantiatingExecutorProvider executorProvider;
+  private boolean rpcIsClosed;
   private final SpannerStub spannerStub;
   private final InstanceAdminStub instanceAdminStub;
   private final DatabaseAdminStub databaseAdminStub;
@@ -631,13 +632,19 @@ private GrpcCallContext newCallContext(@Nullable Map<Option, ?> options, String
 
   @Override
   public void shutdown() {
+    this.rpcIsClosed = true;
     this.spannerStub.close();
     this.instanceAdminStub.close();
     this.databaseAdminStub.close();
     this.spannerWatchdog.shutdown();
     this.executorProvider.shutdown();
   }
 
+  @Override
+  public boolean isClosed() {
+    return rpcIsClosed;
+  }
+
   /**
    * A {@code ResponseObserver} that exposes the {@code StreamController} and delegates callbacks to
    * the {@link ResultStreamConsumer}.

google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java

@@ -233,4 +233,6 @@ PartitionResponse partitionRead(PartitionReadRequest request, @Nullable Map<Opti
       throws SpannerException;
 
   public void shutdown();
+
+  boolean isClosed();
 }

google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java

@@ -21,6 +21,8 @@
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 
+import com.google.cloud.NoCredentials;
+import com.google.cloud.ServiceRpc;
 import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.cloud.spanner.spi.v1.SpannerRpc;
 import java.util.HashMap;
@@ -181,4 +183,49 @@ public Void call() throws Exception {
           }
         });
   }
+
+  @Test
+  public void testSpannerClosed() throws InterruptedException {
+    SpannerOptions options = createSpannerOptions();
+    Spanner spanner1 = options.getService();
+    Spanner spanner2 = options.getService();
+    ServiceRpc rpc1 = options.getRpc();
+    ServiceRpc rpc2 = options.getRpc();
+    // The SpannerOptions object should return the same instance.
+    assertThat(spanner1 == spanner2, is(true));
+    assertThat(rpc1 == rpc2, is(true));
+    spanner1.close();
+    // The SpannerOptions should now no longer be valid.
+    try {
+      options.getService();
+      fail("missing expected exception");
+    } catch (IllegalStateException e) {
+      // This is the expected exception.
+    }
+    // The SpannerOptions should now no longer be valid.
+    try {
+      options.getRpc();
+      fail("missing expected exception");
+    } catch (IllegalStateException e) {
+      // This is the expected exception.
+    }
+    // Creating a copy of the SpannerOptions should result in new instances.
+    options = options.toBuilder().build();
+    spanner1 = options.getService();
+    rpc1 = options.getRpc();
+    assertThat(spanner1 == spanner2, is(false));
+    assertThat(rpc1 == rpc2, is(false));
+    spanner2 = options.getService();
+    rpc2 = options.getRpc();
+    assertThat(spanner1 == spanner2, is(true));
+    assertThat(rpc1 == rpc2, is(true));
+    spanner1.close();
+  }
+
+  private SpannerOptions createSpannerOptions() {
+    return SpannerOptions.newBuilder()
+        .setProjectId("[PROJECT]")
+        .setCredentials(NoCredentials.getInstance())
+        .build();
+  }
 }