Merge pull request #2029 from murgatroid99/grpc-js_local_pool_shutdown

grpc-js: Fix pick first shutdown reference handling

packages/grpc-js/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@grpc/grpc-js",
-  "version": "1.5.2",
+  "version": "1.5.3",
   "description": "gRPC Library for Node - pure JS implementation",
   "homepage": "https://grpc.io/",
   "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",

packages/grpc-js/src/load-balancer-pick-first.ts

@@ -449,11 +449,15 @@ export class PickFirstLoadBalancer implements LoadBalancer {
   destroy() {
     this.resetSubchannelList();
     if (this.currentPick !== null) {
-      this.currentPick.unref();
-      this.currentPick.removeConnectivityStateListener(
+      /* Unref can cause a state change, which can cause a change in the value
+       * of this.currentPick, so we hold a local reference to make sure that
+       * does not impact this function. */
+      const currentPick = this.currentPick;
+      currentPick.unref();
+      currentPick.removeConnectivityStateListener(
         this.pickedSubchannelStateListener
       );
-      this.channelControlHelper.removeChannelzChild(this.currentPick.getChannelzRef());
+      this.channelControlHelper.removeChannelzChild(currentPick.getChannelzRef());
     }
   }
 

packages/grpc-js/test/test-local-subchannel-pool.ts

@@ -0,0 +1,73 @@
+/*
+ * Copyright 2022 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+import * as assert from 'assert';
+import * as path from 'path';
+import * as grpc from '../src';
+import { sendUnaryData, Server, ServerCredentials, ServerUnaryCall, ServiceClientConstructor, ServiceError } from "../src";
+import { loadProtoFile } from './common';
+
+const protoFile = path.join(__dirname, 'fixtures', 'echo_service.proto');
+const echoService = loadProtoFile(protoFile)
+  .EchoService as ServiceClientConstructor;
+
+describe('Local subchannel pool', () => {
+  let server: Server;
+  let serverPort: number;
+
+  before(done => {
+
+    server = new Server();
+    server.addService(echoService.service, {
+      echo(call: ServerUnaryCall<any, any>, callback: sendUnaryData<any>) {
+        callback(null, call.request);
+      },
+    });
+
+    server.bindAsync(
+      'localhost:0',
+      ServerCredentials.createInsecure(),
+      (err, port) => {
+        assert.ifError(err);
+        serverPort = port;
+        server.start();
+        done();
+      }
+    );
+  });
+
+  after(done => {
+    server.tryShutdown(done);
+  });
+
+  it('should complete the client lifecycle without error', done => {
+    const client = new echoService(
+      `localhost:${serverPort}`,
+      grpc.credentials.createInsecure(),
+      {'grpc.use_local_subchannel_pool': 1}
+    );
+    client.echo(
+      { value: 'test value', value2: 3 },
+      (error: ServiceError, response: any) => {
+        assert.ifError(error);
+        assert.deepStrictEqual(response, { value: 'test value', value2: 3 });
+        client.close();
+        done();
+      }
+    );
+  });
+});