Merge pull request #2456 from murgatroid99/grpc-js_minor_fixes

grpc-js: Fix a couple of things that came up while investigating a memory leak
grpc · Jun 5, 2023 · 25e2845 · 25e2845
2 parents bcd52c1 + 2b455e7
commit 25e2845
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 15 deletions.
diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@grpc/grpc-js",
-  "version": "1.8.14",
+  "version": "1.8.15",
   "description": "gRPC Library for Node - pure JS implementation",
   "homepage": "https://grpc.io/",
   "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",

diff --git a/packages/grpc-js/src/client.ts b/packages/grpc-js/src/client.ts
@@ -323,7 +323,7 @@ export class Client {
     emitter.call = call;
     let responseMessage: ResponseType | null = null;
     let receivedStatus = false;
-    const callerStackError = new Error();
+    let callerStackError: Error | null = new Error();
     call.start(callProperties.metadata, {
       onReceiveMetadata: (metadata) => {
         emitter.emit('metadata', metadata);
@@ -342,19 +342,22 @@ export class Client {
         receivedStatus = true;
         if (status.code === Status.OK) {
           if (responseMessage === null) {
-            const callerStack = getErrorStackString(callerStackError);
+            const callerStack = getErrorStackString(callerStackError!);
             callProperties.callback!(callErrorFromStatus({
               code: Status.INTERNAL,
               details: 'No message received',
               metadata: status.metadata
-            }, callerStack));
+            }, /*callerStack*/''));
           } else {
             callProperties.callback!(null, responseMessage);
           }
         } else {
-          const callerStack = getErrorStackString(callerStackError);
-          callProperties.callback!(callErrorFromStatus(status, callerStack));
+          const callerStack = getErrorStackString(callerStackError!);
+          callProperties.callback!(callErrorFromStatus(status, /*callerStack*/''));
         }
+        /* Avoid retaining the callerStackError object in the call context of
+         * the status event handler. */
+        callerStackError = null;
         emitter.emit('status', status);
       },
     });
@@ -448,7 +451,7 @@ export class Client {
     emitter.call = call;
     let responseMessage: ResponseType | null = null;
     let receivedStatus = false;
-    const callerStackError = new Error();
+    let callerStackError: Error | null = new Error();
     call.start(callProperties.metadata, {
       onReceiveMetadata: (metadata) => {
         emitter.emit('metadata', metadata);
@@ -467,7 +470,7 @@ export class Client {
         receivedStatus = true;
         if (status.code === Status.OK) {
           if (responseMessage === null) {
-            const callerStack = getErrorStackString(callerStackError);
+            const callerStack = getErrorStackString(callerStackError!);
             callProperties.callback!(callErrorFromStatus({
               code: Status.INTERNAL,
               details: 'No message received',
@@ -477,9 +480,12 @@ export class Client {
             callProperties.callback!(null, responseMessage);
           }
         } else {
-          const callerStack = getErrorStackString(callerStackError);
+          const callerStack = getErrorStackString(callerStackError!);
           callProperties.callback!(callErrorFromStatus(status, callerStack));
         }
+        /* Avoid retaining the callerStackError object in the call context of
+         * the status event handler. */
+        callerStackError = null;
         emitter.emit('status', status);
       },
     });
@@ -577,7 +583,7 @@ export class Client {
      * call after that. */
     stream.call = call;
     let receivedStatus = false;
-    const callerStackError = new Error();
+    let callerStackError: Error | null = new Error();
     call.start(callProperties.metadata, {
       onReceiveMetadata(metadata: Metadata) {
         stream.emit('metadata', metadata);
@@ -593,9 +599,12 @@ export class Client {
         receivedStatus = true;
         stream.push(null);
         if (status.code !== Status.OK) {
-          const callerStack = getErrorStackString(callerStackError);
+          const callerStack = getErrorStackString(callerStackError!);
           stream.emit('error', callErrorFromStatus(status, callerStack));
         }
+        /* Avoid retaining the callerStackError object in the call context of
+         * the status event handler. */
+        callerStackError = null;
         stream.emit('status', status);
       },
     });
@@ -673,7 +682,7 @@ export class Client {
      * call after that. */
     stream.call = call;
     let receivedStatus = false;
-    const callerStackError = new Error();
+    let callerStackError: Error | null = new Error();
     call.start(callProperties.metadata, {
       onReceiveMetadata(metadata: Metadata) {
         stream.emit('metadata', metadata);
@@ -688,9 +697,12 @@ export class Client {
         receivedStatus = true;
         stream.push(null);
         if (status.code !== Status.OK) {
-          const callerStack = getErrorStackString(callerStackError);
+          const callerStack = getErrorStackString(callerStackError!);
           stream.emit('error', callErrorFromStatus(status, callerStack));
         }
+        /* Avoid retaining the callerStackError object in the call context of
+         * the status event handler. */
+        callerStackError = null;
         stream.emit('status', status);
       },
     });

diff --git a/packages/grpc-js/src/load-balancing-call.ts b/packages/grpc-js/src/load-balancing-call.ts
@@ -216,14 +216,18 @@ export class LoadBalancingCall implements Call {
         break;
       case PickResultType.DROP:
         const {code, details} = restrictControlPlaneStatusCode(pickResult.status!.code, pickResult.status!.details);
-        this.outputStatus({code, details, metadata: pickResult.status!.metadata}, 'DROP');
+        setImmediate(() => {
+          this.outputStatus({code, details, metadata: pickResult.status!.metadata}, 'DROP');
+        });
         break;
       case PickResultType.TRANSIENT_FAILURE:
         if (this.metadata.getOptions().waitForReady) {
           this.channel.queueCallForPick(this);
         } else {
           const {code, details} = restrictControlPlaneStatusCode(pickResult.status!.code, pickResult.status!.details);
-          this.outputStatus({code, details, metadata: pickResult.status!.metadata}, 'PROCESSED');
+          setImmediate(() => {
+            this.outputStatus({code, details, metadata: pickResult.status!.metadata}, 'PROCESSED');
+          });
         }
         break;
       case PickResultType.QUEUE: