diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 4d4ffdb7..1c4baede 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.8.18", + "version": "1.8.19", "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/server-call.ts b/packages/grpc-js/src/server-call.ts index 894d0f76..9db6b51d 100644 --- a/packages/grpc-js/src/server-call.ts +++ b/packages/grpc-js/src/server-call.ts @@ -962,8 +962,8 @@ export class Http2ServerCallStream< } getPeer(): string { - const socket = this.stream.session.socket; - if (socket.remoteAddress) { + const socket = this.stream.session?.socket; + if (socket?.remoteAddress) { if (socket.remotePort) { return `${socket.remoteAddress}:${socket.remotePort}`; } else { diff --git a/packages/grpc-js/src/transport.ts b/packages/grpc-js/src/transport.ts index d5df295b..aa17161f 100644 --- a/packages/grpc-js/src/transport.ts +++ b/packages/grpc-js/src/transport.ts @@ -108,7 +108,12 @@ class Http2Transport implements Transport { /** * Timer reference for timeout that indicates when to send the next ping */ - private keepaliveIntervalId: NodeJS.Timer; + private keepaliveTimerId: NodeJS.Timer | null = null; + /** + * Indicates that the keepalive timer ran out while there were no active + * calls, and a ping should be sent the next time a call starts. + */ + private pendingSendKeepalivePing = false; /** * Timer reference tracking when the most recent ping will be considered lost */ @@ -169,10 +174,8 @@ class Http2Transport implements Transport { } else { this.keepaliveWithoutCalls = false; } - this.keepaliveIntervalId = setTimeout(() => {}, 0); - clearTimeout(this.keepaliveIntervalId); if (this.keepaliveWithoutCalls) { - this.startKeepalivePings(); + this.maybeStartKeepalivePingTimer(); } this.subchannelAddressString = subchannelAddressToString(subchannelAddress); @@ -375,6 +378,14 @@ class Http2Transport implements Transport { this.disconnectListeners.push(listener); } + private clearKeepaliveTimer() { + if (!this.keepaliveTimerId) { + return; + } + clearTimeout(this.keepaliveTimerId); + this.keepaliveTimerId = null; + } + private clearKeepaliveTimeout() { if (!this.keepaliveTimeoutId) { return; @@ -383,7 +394,19 @@ class Http2Transport implements Transport { this.keepaliveTimeoutId = null; } - private sendPing() { + private canSendPing() { + return ( + this.keepaliveTimeMs > 0 && + (this.keepaliveWithoutCalls || this.activeCalls.size > 0) + ); + } + + private maybeSendPing() { + this.clearKeepaliveTimer(); + if (!this.canSendPing()) { + this.pendingSendKeepalivePing = true; + return; + } if (this.channelzEnabled) { this.keepalivesSent += 1; } @@ -402,6 +425,7 @@ class Http2Transport implements Transport { (err: Error | null, duration: number, payload: Buffer) => { this.keepaliveTrace('Received ping response'); this.clearKeepaliveTimeout(); + this.maybeStartKeepalivePingTimer(); } ); } catch (e) { @@ -411,25 +435,36 @@ class Http2Transport implements Transport { } } - private startKeepalivePings() { - if (this.keepaliveTimeMs < 0) { + /** + * Starts the keepalive ping timer if appropriate. If the timer already ran + * out while there were no active requests, instead send a ping immediately. + * If the ping timer is already running or a ping is currently in flight, + * instead do nothing and wait for them to resolve. + */ + private maybeStartKeepalivePingTimer() { + if (!this.canSendPing()) { return; } - this.keepaliveIntervalId = setInterval(() => { - this.sendPing(); - }, this.keepaliveTimeMs); - this.keepaliveIntervalId.unref?.(); - /* Don't send a ping immediately because whatever caused us to start - * sending pings should also involve some network activity. */ + if (this.pendingSendKeepalivePing) { + this.pendingSendKeepalivePing = false; + this.maybeSendPing(); + } else if (!this.keepaliveTimerId && !this.keepaliveTimeoutId) { + this.keepaliveTrace( + 'Starting keepalive timer for ' + this.keepaliveTimeMs + 'ms' + ); + this.keepaliveTimerId = setTimeout(() => { + this.maybeSendPing(); + }, this.keepaliveTimeMs).unref?.(); + } + /* Otherwise, there is already either a keepalive timer or a ping pending, + * wait for those to resolve. */ } - /** - * Stop keepalive pings when terminating a connection. This discards the - * outstanding ping timeout, so it should not be called if the same - * connection will still be used. - */ private stopKeepalivePings() { - clearInterval(this.keepaliveIntervalId); + if (this.keepaliveTimerId) { + clearTimeout(this.keepaliveTimerId); + this.keepaliveTimerId = null; + } this.clearKeepaliveTimeout(); } @@ -437,20 +472,17 @@ class Http2Transport implements Transport { this.activeCalls.delete(call); if (this.activeCalls.size === 0) { this.session.unref(); - if (!this.keepaliveWithoutCalls) { - this.stopKeepalivePings(); - } } } private addActiveCall(call: Http2SubchannelCall) { - if (this.activeCalls.size === 0) { + this.activeCalls.add(call); + if (this.activeCalls.size === 1) { this.session.ref(); if (!this.keepaliveWithoutCalls) { - this.startKeepalivePings(); + this.maybeStartKeepalivePingTimer(); } } - this.activeCalls.add(call); } createCall(